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.

Sign up for GitHub

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

Jump to bottom

Fix: Enrolment Overview Error Miss Reporting #2141

Merged
merged 2 commits into from
Apr 25, 2022
Merged

Fix: Enrolment Overview Error Miss Reporting #2141

merged 2 commits into from
Apr 25, 2022

Conversation

hogmann
Copy link
Contributor

@hogmann hogmann commented Apr 25, 2022

Enrolment errors should only represent states that mean the overview is false, not just helper methods for deciding error messaging. Not having a cert is not an error but just used in the review page for message logic.

@hogmann
Fix: Enrolment Overview Error Miss Reporting
45cc38a 
Enrolment errors should only represent states that mean the overview is false, not just helper methods for deciding error messaging. Not having a cert is not an error but just used in the review page for message logic.
@hogmann hogmann added Bug Something isn't working Frontend Updates to the frontend of the application Ready For Review PR is ready for code review labels Apr 25, 2022
@hogmann hogmann requested a review from Paahn April 25, 2022 18:39
@hogmann hogmann temporarily deployed to Development April 25, 2022 18:40 Inactive
@hogmann hogmann temporarily deployed to Development April 25, 2022 21:40 Inactive
@hogmann hogmann temporarily deployed to Development April 25, 2022 21:40 Inactive
@hogmann hogmann temporarily deployed to Development April 25, 2022 21:40 Inactive
@hogmann hogmann temporarily deployed to Development April 25, 2022 21:47 Inactive
Paahn
Paahn approved these changes Apr 25, 2022
View reviewed changes
Copy link
Collaborator

@Paahn Paahn left a comment

Choose a reason for hiding this comment

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

Went through an enrolment without a cert and didn't get error on overview, LGTM unless there is anything else to test

@hogmann hogmann merged commit 2c7779e into develop Apr 25, 2022
@hogmann hogmann deleted the fix/overview-filtering branch April 25, 2022 22:47
hogmann added a commit that referenced this pull request Apr 25, 2022
@hogmann
Fix: Enrolment Overview Error Miss Reporting (#2141)
62fbc79 
Enrolment errors should only represent states that mean the overview is false, not just helper methods for deciding error messaging. Not having a cert is not an error but just used in the review page for message logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Paahn Paahn Paahn approved these changes

Assignees
No one assigned
Labels
Bug Something isn't working Frontend Updates to the frontend of the application Ready For Review PR is ready for code review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hogmann @Paahn