-
-
Notifications
You must be signed in to change notification settings - Fork 473
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
added additional logic to contact_topic_answers to improve discoverab… #5865
Conversation
…ility for case topics if admin has not set any court report topics
There was a problem hiding this 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 :)
… different users view the page and no contact topics are available
@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. |
There was a problem hiding this 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.
03f6216
to
bd9c281
Compare
, moved volunteer and supervisor tests under GET /show to improve future test writing
…equirements, and simplify logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice job!
Looks good! Sorry about the flaky spec.
|
@josephmsmith this is super minor thing but FYI you have a few commits where the wrote a really long first line of they look like |
Noted! Thank you for all your help @elasticspoon |
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 :)
Feelings gif (optional)
_What gif best describes your feeling working on this issue? https://giphy.com/
