Skip to content

Conversation

@gnprice
Copy link
Member

@gnprice gnprice commented Oct 10, 2025

Fixes #1732.

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Oct 10, 2025
@chrisbobbe
Copy link
Collaborator

Thanks, LGTM! There's an existing PR for the get-temporary-URL binding: #1780. But it's not as close to merge as this one is, so please merge at will. The only nit I think still might apply from my last review there, in #1780 (comment) :

The new endpoint isn't really about a message, so instead of messages.dart, let's put it in a new file like lib/api/route/uploads.dart.

But I don't feel strongly about it.

@gnprice
Copy link
Member Author

gnprice commented Oct 16, 2025

Thanks for the review!

The new endpoint isn't really about a message, so instead of messages.dart, let's put it in a new file like lib/api/route/uploads.dart.

Ah indeed. The same logic applies to the existing binding for upload-file, I think. It'd be reasonable to move them both to a new such file.

OTOH the current way matches the organization in the API docs' left nav bar: both these endpoints are included under "Messages".

@gnprice gnprice force-pushed the pr-auth-upload-link branch from f9a3a6a to efc41b3 Compare October 16, 2025 23:59
@chrisbobbe
Copy link
Collaborator

OTOH the current way matches the organization in the API docs' left nav bar: both these endpoints are included under "Messages".

Sure, that's good enough reason to leave them where they are. Ship it! 🙂

@gnprice gnprice merged commit efc41b3 into zulip:main Oct 16, 2025
1 check passed
@gnprice gnprice deleted the pr-auth-upload-link branch October 16, 2025 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer review PR ready for review by Zulip maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open uploaded files without logging in again

2 participants