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

added additional logic to contact_topic_answers to improve discoverab… #5865

Conversation

josephmsmith
Copy link
Collaborator

What github issue is this PR for, if any?

Resolves #5775

What changed, and why?

I added additional logic to contact_topic_answers to improve discoverability for case topics if admin has not set any court report topics. This was important because users did not know admins needed to set court report topics in order for the feature to be accessible by volunteers, supervisors, and admins.

How is this tested? (please write tests!) 💖💪

This was tested by removing all topics from Court Report Topics as an admin, then logging in as each user type and confirming what verbiage was displayed as it needed to be different for admins versus volunteers and supervisors.

I then logged in as an admin then set court report topics and repeated the process of logging in as each user type to confirm court report topics would be visible.

For each test, I inspected the code and simulated being in a different browser type (mobile, tablet, laptop) to confirm the CSS classes conformed with the rest of the page.\

Screenshots please :)

court_report_NO_topics_admin_laptop
court_report_NO_topics_mobile_admin
court_report_NO_topics_volunteer_laptop
court_report_NO_topics_volunteer_mobile
court_report_topic_mobile_admin
court_report_topic_tablet_admin
court_report_topics_laptop_supervisor

Feelings gif (optional)

_What gif best describes your feeling working on this issue? https://giphy.com/
![noice](https://i.giphy.com/media/v1.Y2lkPTc5MGI3NjExdjBmMmFrd25wMmI5ZjAyZjU3dWljcXNkYTMzc3h0ZTN2bnJqZHNndyZlcD12MV9pbnRlcm5hbF9naWZfYnlfaWQmY3Q9Zw/4KF85OSbyjVOfyjksJ/giphy.gif)

…ility for case topics if admin has not set any court report topics
Copy link
Collaborator

@compwron compwron left a comment

Choose a reason for hiding this comment

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

Looks promising! I see an erb lint failure. Please add a test :)

@github-actions github-actions bot added ruby Pull requests that update Ruby code Tests! 🎉💖👏 labels Jun 30, 2024
@josephmsmith
Copy link
Collaborator Author

@compwron can you help guide on next steps for the two failures?

@elasticspoon
Copy link
Collaborator

@compwron can you help guide on next steps for the two failures?

Those are false positives. Unfortunately there are issues with some tests timing out and looking like failures.

So your pr passes the test suite.

@josephmsmith josephmsmith marked this pull request as ready for review July 1, 2024 18:34
Copy link
Collaborator

@elasticspoon elasticspoon left a comment

Choose a reason for hiding this comment

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

Nice tests! This is pretty close to what we are looking for. See comments for suggestions.

spec/requests/case_contacts/form_spec.rb Show resolved Hide resolved
spec/requests/case_contacts/form_spec.rb Outdated Show resolved Hide resolved
app/views/case_contacts/form/details.html.erb Outdated Show resolved Hide resolved
@josephmsmith josephmsmith force-pushed the 5775-improve-discoverabilty-for-creating-and-editting-court-report-topics branch from 03f6216 to bd9c281 Compare July 3, 2024 01:52
, moved volunteer and supervisor tests under GET /show to improve future test writing
Copy link
Collaborator

@elasticspoon elasticspoon left a comment

Choose a reason for hiding this comment

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

LGTM! Nice job!

@compwron
Copy link
Collaborator

compwron commented Jul 5, 2024

Looks good! Sorry about the flaky spec.


  1) case_contacts/index with case contacts without filter displays the contact type groups
     Failure/Error: expect(page).to have_text(case_contacts[2].contact_groups_with_types.keys.first)
       expected to find text "Group 1388" in "Group 1389\nDraft"

@compwron compwron merged commit e5d68e7 into main Jul 5, 2024
18 of 19 checks passed
@compwron compwron deleted the 5775-improve-discoverabilty-for-creating-and-editting-court-report-topics branch July 5, 2024 19:26
@elasticspoon
Copy link
Collaborator

@josephmsmith this is super minor thing but FYI
in git commits you typically want to write in the first line main point of the commit in 80 chars or less.
then have an empty line
the write the body of the commit which has move context.

you have a few commits where the wrote a really long first line of they look like
image
vs

image

@josephmsmith
Copy link
Collaborator Author

Noted! Thank you for all your help @elasticspoon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
erb ruby Pull requests that update Ruby code Tests! 🎉💖👏
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve discoverabilty for creating and editting Court Report Topics
3 participants