Skip to content

Conversation

@seanaye
Copy link
Contributor

@seanaye seanaye commented Dec 2, 2025

Summary

This PR fixes an issue where soup would fail if the user does not have an email link.
There is unit tests for the changed behaviour

  • support soup when email link not found
  • add tests

Screenshots, GIFs, and Videos

@seanaye seanaye requested a review from a team as a code owner December 2, 2025 23:06
"dep:model_user",
"dep:utoipa",
"email/axum",
"item_filters/schema",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a missing feature right now if you run cargo test -p soup --all-features

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so yes

State(service): State<SoupRouterState<T, U>>,
Cached(MacroUserExtractor { macro_user_id, .. }): Cached<MacroUserExtractor>,
Cached(EmailLinkExtractor(link, _)): Cached<EmailLinkExtractor<U>>,
email_link: Result<Cached<EmailLinkExtractor<U>>, EmailLinkErr>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be cached? if the user enables email won't the link extractor be none even tho there should be one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

axum cache is unique per http request, it means if the handler tries to extract multiple times it will only query the db 1 time for that request

@seanaye seanaye merged commit 22b6bd1 into main Dec 2, 2025
37 checks passed
@seanaye seanaye deleted the seanaye/fix/soup-without-email-link branch December 2, 2025 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants