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

Disable delivery system edit buttons, unless selected #91

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

benmartin-coforma
Copy link
Contributor

Description

At the bottom of each measure page is a table of delivery systems, with an Edit button for each row. But users are only required to report on the delivery system(s) they indicated they would report on, in a prior question. This pull request wires up these buttons' enabled/disabled states to that prior question.

image

In order to make this happen, I added a field called formKey to the delivery system selection radio button. Most inputs will have an auto-generated name like "elements.7"; these radio buttons will now have fixed names. Then I can query React Hook Form by the appropriate name to get the selected delivery system instantly (that is, without waiting for the report to update in the store).

I also changed the values of the radio buttons. Whereas previously we had semantic-kebab-case values, now the values correspond directly the DeliverySystem enum - with multiple systems joined by commas if necessary.

In the future, we may want to add a formKey property to the rest of our elements. I don't love the code I added in Page.tsx to "maybe use the formKey we have, or maybe generate one". I'm not sure how much work would be involved in convincing Typescript that the elements we care about will always have form keys. It's possible that we will be able to remove that code entirely, by restructuring the ComposedElement pattern... somehow. In fact, we may want to do that before merging this PR. I request your opinion, dear reviewer.

Related ticket(s)

CMDCT-4182


How to test

  1. Log in as a state user
  2. Create a QMS report
  3. Go to "Required Measure Results" or "Optional Measure Results"
  4. Select a measure to update which allows for multiple Delivery Systems, such as FASI or LTSS
  5. Note that at the bottom of the page, both rows of the Delivery Systems table have disabled "Edit" buttons
  6. Select a delivery method radio button in the last question
  7. Note that the "Edit" buttons instantly update, to be enabled or disabled as appropriate

Important updates

Note that these changes are not backwards-compatible with existing reports, because of the changes I made to qms.ts


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary

convert to a different template: test → val | val → prod

Copy link

codeclimate bot commented Jan 15, 2025

Code Climate has analyzed commit 92ff026 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 93.6% (0.0% change).

View more on Code Climate.

Copy link
Contributor

@BearHanded BearHanded left a comment

Choose a reason for hiding this comment

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

I also am not sure if there's a better way to do it, but I do feel like it doesn't lock us into anything if we come up with a better idea

Copy link
Contributor

@ajaitasaini ajaitasaini left a comment

Choose a reason for hiding this comment

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

looks great!

@benmartin-coforma benmartin-coforma merged commit 35a9fea into main Jan 22, 2025
19 checks passed
@benmartin-coforma benmartin-coforma deleted the deliv-meth-enable branch January 22, 2025 21:30
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.

3 participants