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

PC-1347: Routing error shown on success page #396

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

samyou-softwire
Copy link
Contributor

Link to Jira ticket

Description

adds a routing exception for the success page

in this case, the exception does nothing, so opted to mark it as None. could have a different way of marking that there's no previous page for success?

Checklist

  • I have made any necessary updates to the documentation
  • I have checked there are no unnecessary IDE warnings in this PR
  • I have checked there are no unintentional line ending changes
  • I have added tests where applicable
  • If I have made any changes to the code, I have used the IDE auto-formatter on it
  • If I have made any changes to Python files (even if not changing any content strings), I have run make extract-translations

@samyou-softwire samyou-softwire self-assigned this Nov 6, 2024
@samyou-softwire samyou-softwire force-pushed the PC-1347-routing-error-shown-on-success-page branch from 6df2fa1 to 29a2040 Compare November 6, 2024 14:57
Copy link
Contributor

@jdgage jdgage left a comment

Choose a reason for hiding this comment

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

Could make it a clearly-named constant to make it obvious that it's a deliberate thing? You might not need the comment then.

return page_name_to_url(session_id, schemas.routing_overrides[page_name]["prev_page"])
prev_page = schemas.routing_overrides[page_name]["prev_page"]
# if there is no previous page (or no back button at all), fall back to doing nothing
return page_name_to_url(session_id, prev_page if prev_page is not None else page_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Although maybe wants explaining what it means to return the current page name from this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have extracted to a constant and provided some more details here

@samyou-softwire samyou-softwire force-pushed the PC-1347-routing-error-shown-on-success-page branch from 29a2040 to 37067b6 Compare November 7, 2024 16:41
@Glenn-Clarke Glenn-Clarke merged commit 44dd6c1 into develop Nov 12, 2024
4 checks passed
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.

3 participants