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

Show retry button on enrollment/retry page #1787

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Nov 16, 2023

fix #1755

What this PR does

  • Pass retry_button as True to the enrollment/retry page, so that the button appears

@machikoyasuda machikoyasuda requested a review from a team as a code owner November 16, 2023 00:49
@machikoyasuda machikoyasuda self-assigned this Nov 16, 2023
@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Nov 16, 2023
Copy link

github-actions bot commented Nov 16, 2023

Coverage report

The coverage rate went from 90.87% to 90.87% ➡️
The branch rate is 85%.

None of the new lines are part of the tested code. Therefore, there is no coverage data about them.

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

This definitely works, but I'm curious why we even have this retry_button context and the {% if ... %} check in our template.

Don't we always want to show a retry button on the retry page? Could we just remove this context variable and the template check?

@machikoyasuda
Copy link
Member Author

machikoyasuda commented Nov 16, 2023

@thekaveman Yeah I wondered the same thing. As long as this Retry page/template is not called from other methods, I don't think we would need the check. Should I just remove?

@angela-tran
Copy link
Member

I looked through the Git history a little, and it seems like in 4c49895 we just forgot to remove if retry_button from the template.

@thekaveman
Copy link
Member

@thekaveman Yeah I wondered the same thing. As long as this Retry page/template is not called from other methods, I don't think we would need the check. Should I just remove?

Thanks for the find @angela-tran.

Yep @machikoyasuda I think we are good to remove this.

@machikoyasuda machikoyasuda merged commit 3446877 into dev Nov 16, 2023
11 checks passed
@machikoyasuda machikoyasuda deleted the feat/1755-retry branch November 16, 2023 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retry button is missing
3 participants