Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle for invalid referer for embedded page service #9407

Conversation

Philwi
Copy link

@Philwi Philwi commented Jul 11, 2022

What? Why?

Closes #9355

In some cases the referer of the current url is invalid and not a valid url. In these cases the current behaviour throws an unhandled exception and the page could not been visit.

What should we test?

  • I could not reproduce it in Chrome or in Firefox 👎

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

@jibees
Copy link
Contributor

jibees commented Jul 12, 2022

This PR contains some commit (around packages upgrade) that shouldn't appear it. Could you please rebase/squash your PR? Thanks a lot! 🙏

@Philwi Philwi force-pushed the handle-for-invalid-referer-for-embedded-page-service branch from cb5e608 to 6291e3c Compare July 13, 2022 16:56
@Philwi
Copy link
Author

Philwi commented Jul 13, 2022

This PR contains some commit (around packages upgrade) that shouldn't appear it. Could you please rebase/squash your PR? Thanks a lot! pray

Yes, for sure! :)

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Nice!

@filipefurtad0 filipefurtad0 self-assigned this Aug 17, 2022
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Aug 17, 2022
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Aug 17, 2022

Hey @Philwi ,

Thanks - I could not reproduce the bug either.
I'm not sure how I could test your code change. I've deployed it, navigated around, typed in some incorrect URLs but as described in the issue, but yeah the browser (Firefox) corrected it. Also and using Browserstack tried it in Microsoft Explorer, but noticed nothing unusual.

From what I gather the PR adds a safeguard against incorrect URLs - provided we have them. Having nothing else to check I'd say this is good to merge. If something crosses your mind don't hesitate and do let me know 👍

Thanks again!

@filipefurtad0 filipefurtad0 merged commit c11b7ed into openfoodfoundation:master Aug 17, 2022
@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedding group page with invalid URL raises error
5 participants