Case Contact Form Overhaul

[thejonroberts]

What github issue is this PR for, if any?

Resolves #XXXX - Is there an issue for this yet?

Please review/merge these PRs first:

• [ ] AdditionalExpenses and ContactTopicAnswer controllers & policies #6060
• CaseContact validates usage and spec changes - case contact form prep #6061
• Update stimulus rails nested-form & switch to namespaced version #6057

They include some of the changes here, but can be merged with no changes to existing behavior. That will make review of this giant PR a little easier.

What changed, and why?

Total overhaul of CaseContact form. Reworked to be in one page. This was accomplished by reducing the wicked wizard steps to one - the details step.

Maintaining the autosave feature was complicated! Before, notes were the only thing that could change during an autosave, but now everything is in the form that gets sent during the autosave update. Therefore, nested_attributes_for contact_topic_answers and additional_expenses required a stimulus NestedForm controller that could create and destroy records -- otherwise, autosave would make new records for every save, or try to delete records that had already been removed in the prior update. This also required creating controllers & routes for those json requests.

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

I had to rewrite request and system specs in the process. Request spec was made to be more typical, testing request responses for certain parameters, and not checking page content - I moved those into system specs. I got rid of the system/create spec and moved those tests to new or edit specs as applicable (I'm not sure how create was different from 'new' in a system spec). There were also specs asserting things about case show to that spec rather than the form specs, which I moved over to the case show specs.

Also added request and policy specs for the new controller routes, as mentioned.

Additional Notes

There are a few things from the design requirements that have not been met. I have pending specs for those (which may need to be removed?). I recommend creating issues and handling them separately. They are:

• Limiting to one answer per ContactTopic, as well as limiting number of topic answers to number of topics
• Showing ContactTopic description in tooltip. This will require swapping the tooltip out on topic select, and I thought there is a lot of javascript logic already.
• There may be some other minor things in the design document.
• I also think it would be good to make an issue for better accessibility. I tried to get the basics out of the way, but one thing that really bothers me is that the form is VERY difficult to tab-navigate through.

Screenshots please :)

Feelings gif (optional)

_What gif best describes your feeling working on this issue? https://giphy.com/

[bcastillo32]

wait WOW @thejonroberts thank you so much for jamming on this. You have made my month 😂

[bcastillo32]

@thejonroberts thank you for the feedback on the following:

• Requiring ContactType (validation)
• Limiting to one answer per ContactTopic, as well as limiting number of topic answers to number of topics
• Showing ContactTopic description in tooltip. This will require swapping the tooltip out on topic select, and I thought there is a lot of javascript logic already.
• There may be some other minor things in the design document.
• I also think it would be good to make an issue for better accessibility. I tried to get the basics out of the way, but one thing that really bothers me is that the form is VERY difficult to tab-navigate through.

I can create some sep tix for these. As fo this PR @elasticspoon do we need an issue to tie this PR to?

[bcastillo32]

@thejonroberts thank you for the feedback on the following:

• Requiring ContactType (validation)
• Limiting to one answer per ContactTopic, as well as limiting number of topic answers to number of topics
• Showing ContactTopic description in tooltip. This will require swapping the tooltip out on topic select, and I thought there is a lot of javascript logic already.
• There may be some other minor things in the design document.
• I also think it would be good to make an issue for better accessibility. I tried to get the basics out of the way, but one thing that really bothers me is that the form is VERY difficult to tab-navigate through.

I can create some sep tix for these. As fo this PR @elasticspoon do we need an issue to tie this PR to?

@thejonroberts just chatted with Mallory (designer) and confirmed that we should be Requiring ContactType - are you able to do a "hot fix" here or would you like a new ticket?

[thejonroberts]

@bcastillo32 I can add the ContactType requirement here.

[bcastillo32]

@bcastillo32 I can add the ContactType requirement here.

Ok great. Thank you so much @thejonroberts !

[elasticspoon]

Hey! Honestly, this is really good. I really liked the fallback to additional notes when an org has no contact topics set.

I haven't looked at the code too closely but mainly clicked around a bunch to do some higher level testing:

Blockers

• delete button for notes does not seem to work?
  • I start a new case contact, add a topic answer, fill it in, reload the page, delete topic answer => is disappears from the page => reload page => its back
  • I see a "you are not authorized' banner at the top of the page (I am admin creating a case contact)
  • logs show ActionController::RoutingError (No route matches [DELETE] "/"):
  • should also probably skip app/controllers/application_controller.rb:39:in set_active_banner' for an api only request
• creating a contact topic answer:
  • also tries to set_active_banner
  • have this in the logs

ActionController::UnknownFormat (SupervisorsController#index is missing a template for this request format and variant.
request.formats: ["application/json"]
request.variant: []):

• I think drafts are broken?
  • logged in as a supervisor, picked a case, new case contact
  • filled in some stuff, did not submit
  • went back to page with all contacts for a case and contact was not there
• I seem to be getting the unauthorized message on random reloads of the contact form

Nice to have (not blocking)

• add some client side validation to the required fields
• mark the driving reimbursement fields are required
• autosave popup would be nice on other fields as well, just so I know when I am good

[elasticspoon]

@elasticspoon , @FireLemons Regarding the autosave questions and my previous comments:

• I could add a data-action="input->autosave#save" on every single input to fire off an autosave on every change

  • Where to show notifications? Top of every section? No design for these, my want to think on it.

• OR we could just autosave every X seconds

  • will save a lot of template actions, and make fewer server requests... unless someone walks away from their computer, and it just sends the same update over and over...

• Maybe a combo, autosave on any input, but stretch out the debounce time to like 30 seconds or a minute?

I think a decent and easy enough solution for now would be to add autosave actions for Additional Expense Description inputs. The expenses may also be difficult to remember/re-enter if you get logged out. I'll add that. I think everything else in the form is easy enough to re-do if you get logged out, and with notes/expenses at the bottom of the form, you will hit an autosave after you have filled them in (typically).

Does that sound like a good approach to y'all?

Yea. I'm good with that as an initial approach and we can see in the future if we need to change that.

[elasticspoon]

Most of my stuff is just small comments but the drafts on the case details page (/casa_cases/:case_slug) don't appear, which is actually blocking.

volunteer.mp4

[thejonroberts]

Most of my stuff is just small comments but the drafts on the case details page (/casa_cases/:case_slug) don't appear, which is actually blocking.

@elasticspoon, that cannot be done currently in the app! I tried it out on main branch to confirm.

We don't set CaseContact.casa_case_id until the final submission of the contact. Selected case(s) are in draft_case_id column only. The CasaCase has_many :case_contacts relationship is not there yet, so Case show view is unaware of the drafts. You can only see draft contacts from the CaseContact index (/case_contacts).

[thejonroberts]

@MalloryPriceDesign thanks so much for the detailed feedback! I have reworked the form quite a bit and it is much better now! Notes:

To address in this PR:

• Date auto-populating:

I have made the date input blank on load. Note that there is some weird behavior in Safari browsers - The date input's value is null, but it is displayed as today's date. So user may think it is set, but value is null and will error if form is submitted as is. This could be confusing, so I added basic browser validation to require an input before submit... I think that feedback could probably be improved, depends on the browser implementation.

I also could not get the CSS for input placeholder text color to work consistently for all inputs, I think due to our bootstrap setup. These issues would make good improvement tickets?

• Days since the last contact:
• Contact Medium options:
• Including One Discussion Note:
  When the page is initialized, we should include one discussion topic and note. Users report wanting to be able to write their notes down quickly, so having a textbox already available should save them a click. Also, can you confirm that these Discussion Topics are being pulled from the organization's configuration? We'll be using these values to map the notes to their corresponding section in the generated court report.

• There is one note shown on load now, unless editing an active contact that had no answers, or organization has no contact topics.
• These topics are from Organization's contact topics. If there are none available, a disclaimer is shown (as previously), and an 'Additional Notes' field is rendered for contact.notes column.

• Limit to one entry per topic in Notes:
  If it's not too hard, can we limit the number of entries for each topic to one? I'm assuming this should also help our handling of these topics on the backend when generating reports so we don't have to stitch multiple entries per topic together.

• I haven't done this yet but will try. I thought of a solution that is likely workable - disabling options that are currently selected... adding and removing completely from list of options is more complicated. Going to attempt it now.
• I think case contact report will be affected. (uses index_by(&:contact_topic_id): last topic answer will be shown, not others)
• EDIT: this is now implemented - topics that have been selected are disabled in other selects, 'Add..' button disabled after answers match the number of topics

• Reimbursement Visibility
  Can you confirm that the whole reimbursement card is only visible if the user's administrator has enabled reimbursement? Additionally, I cannot see the "Other Expense Amount" and "Other Expense Details" inputs in this section.

Yes, the entire Reimbursement section card is not rendered unless organization driving reimbursement is enabled.

1. Are we including the "Other Expense Amount" and "Other Expense Details" input boxes? If not, let's make a ticket for them
2. Are we maintaining functionality where if an admin has not enabled reimbursement for this org, this whole box isn't viewable to the volunteer

Expenses are behind another feature flag (also in Organization Edit available to admin), so they are not shown unless Driving Reimbursement & Expense reimbursement are both enabled. Note that this means creator must request driving reimbursement to request Other Expenses... So you can have no reimbursement, driving reimbursement only, or driving & expense reimbursement. But not only Expense reimbursement.

Additional feedback

Overall UI Feedback

Reworked classes and CSS to be much more consistent with design. Pretty close I think, the Figma files were extremely helpful, thank you.

Notes Section

1. The "Discussion Topic" should not be populated by default, with the "Select a discussion topic" alt text showing.

This is a little more complex than I want to deal with right now. Because of a database constraint on contact_topic_id (not null), we need a topic id when a new contact topic answer (Discussion Note) is added to the form. I think this could be implemented if the database constraint is removed, which would then allow us to require the Topic ID conditionally. Alternatively, could do some (overly complicated imo) javascript manipulations? Regardless, I think it is better left for future work.

EDIT: I am going to try just removing the database constraint... This would actually help with the one entry per topic mentioned above. This is implemented now.

Reimbursement Section
4. Is it possible to make the height of the "total miles driven" and "current address" input boxes the same?

The only way to do this (easily) means that it can only be a one line entry, as opposed to a more traditional address format, is that okay? Will wait to make the change until that is confirmed.

[thejonroberts]

@MalloryPriceDesign - ready for re-review, see above, edited some notes I made earlier. (may need to be deployed to the environment you were looking at, I'm not sure how that was setup)

@elasticspoon - ready for re-review, with the caveat that I have not addressed the CasaCase show page draft issue, as I don't think that was a feature prior to this work, unless I'm missing something.

[MalloryPriceDesign]

Reimbursement Section
4. Is it possible to make the height of the "total miles driven" and "current address" input boxes the same?

The only way to do this (easily) means that it can only be a one line entry, as opposed to a more traditional address format, is that okay? Will wait to make the change until that is confirmed.

Let's keep this as-is, I think the users will probably expect the traditional address format instead of a one-line format.

@MalloryPriceDesign - ready for re-review, see above, edited some notes I made earlier. (may need to be deployed to the environment you were looking at, I'm not sure how that was setup)

That's great! I used Codespaces to review previously. However, after running git pull and building the app again in my previous Codespace, I got an error. I restarted with a fresh Codespace and am getting the same error:

This page isn’t working
supreme-disco-rj9q4grx54jfppxg-3000.app.github.dev is currently unable to handle this request.
HTTP ERROR 502

I'll take a look again later.

[elasticspoon]

Most of my stuff is just small comments but the drafts on the case details page (/casa_cases/:case_slug) don't appear, which is actually blocking.

@elasticspoon, that cannot be done currently in the app! I tried it out on main branch to confirm.

We don't set CaseContact.casa_case_id until the final submission of the contact. Selected case(s) are in draft_case_id column only. The CasaCase has_many :case_contacts relationship is not there yet, so Case show view is unaware of the drafts. You can only see draft contacts from the CaseContact index (/case_contacts).

You are right. My flabbers are ghasted. I totally thought that was something that could be done.

[elasticspoon]

@thejonroberts This is pretty tremendous work man. You are a saint to doing all this work and being so thorough about it. I don't see any blocking issues. I'm sure something will come up but I'm fine to send this as is.

I'll let @MalloryPriceDesign chime in if she has any further input.

[MalloryPriceDesign]

Looks amazing! I rebuilt my Codespace container and it's working now!

This looks good! The only change I would suggest is to enable the calendar dropdown to display on the initial click into the date box. Currently, the user has to go all the way to the right calendar icon to click to open the calendar. Is that something that could be easily changed? If not, I don't think this should block merging and can always be addressed in another ticket.

[thejonroberts]

Thank you, Mallory...

This looks good! The only change I would suggest is to enable the calendar dropdown to display on the initial click into the date box. Currently, the user has to go all the way to the right calendar icon to click to open the calendar. Is that something that could be easily changed? If not, I don't think this should block merging and can always be addressed in another ticket.

This is browser dependent -- Safari opens date picker on click for example. I think any changes to browser input behavior should be separate to make sure time can be spent ensuring cross-browser behaviors and accessibility considerations are maintained, a lot of gotchas can pop up!

[compwron]

wow wow wow!!

[bcastillo32]

thank you everyone for the work on this! This is amazing and feedback on the codespace was very positive!