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 reason for challenge to admins when partnership already exists #4892

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

leoapost
Copy link
Contributor

@leoapost leoapost commented Jun 5, 2024

Context

  • Ticket: N/A

Enhance context for displaying the "This relationship already exists" page when Admins attempt to change participants' LP/DP in an already existing and challenged partnership.

Changes proposed in this pull request

  • Added the challenged date and challenge reason to the view

Guidance to review

Before After
Screenshot from 2024-06-05 13-42-46 Screenshot from 2024-06-05 14-21-04

Copy link

github-actions bot commented Jun 5, 2024

Review app deployed to https://cpd-ecf-review-4892-web.test.teacherservices.cloud

@leoapost leoapost force-pushed the enhance-admin-existing-partnership-view branch 3 times, most recently from 9e11008 to 6114d4a Compare June 5, 2024 13:26
@leoapost leoapost force-pushed the enhance-admin-existing-partnership-view branch from 6114d4a to 3630a60 Compare June 5, 2024 13:48
<% end %>
<% summary_list.with_row do |row| %>
<% row.with_key { "Challenge reason" } %>
<% row.with_value { @wizard.existing_partnership.challenge_reason } %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we translate these into plain English for display instead of underscored code-y values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only super users will see this information, and they are likely to reach out to developers for support. So, I thought raw values would be easier for troubleshooting.

Happy to use the English translations instead, but I'll most likely run a search each time in the code to figure out the raw values :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Even something like @wizard.existing_partnership.challenge_reason.humanize is better and probably enough?

To provide more context to super users why the partnership can't be created.
@leoapost leoapost force-pushed the enhance-admin-existing-partnership-view branch from 3630a60 to db01c8f Compare June 5, 2024 14:41
@leoapost leoapost enabled auto-merge June 5, 2024 16:04
@leoapost leoapost added this pull request to the merge queue Jun 5, 2024
Merged via the queue into main with commit 36d8dcd Jun 5, 2024
31 checks passed
@leoapost leoapost deleted the enhance-admin-existing-partnership-view branch June 5, 2024 16:43
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.

2 participants