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

6180 prescribed quantity #6345

Open
wants to merge 28 commits into
base: develop
Choose a base branch
from
Open

Conversation

GeronimoJohn
Copy link
Contributor

@GeronimoJohn GeronimoJohn commented Jan 29, 2025

Fixes #6180

πŸ‘©πŸ»β€πŸ’» What does this PR do?

This update introduces a new field in the Line Edit View to record the prescribed quantity of an item. The field is editable and does not perform any calculations. The data entered in this field is saved to the database and synchronized with mSupply.

Screen.Recording.2025-01-31.at.12.08.23.PM.mov

πŸ’Œ Any notes for the reviewer?

Left a comment for an improvement that will be worked on a different PR

πŸ§ͺ Testing

  • Edit or create a new prescription
  • A field called Prescribed Quantity is on the screen
  • The field should either be pre-filled with a default value or zero if there's none
  • The field can be updated to our desired prescribed quantity

πŸ“ƒ Documentation

  • Part of an epic: documentation will be completed for the feature as a whole
  • No documentation required: no user facing changes or a bug fix which isn't a change in behaviour
  • These areas should be updated or checked:
    1.
    2.

@github-actions github-actions bot added this to the v2.6.0 milestone Jan 29, 2025
@github-actions github-actions bot added Team Piwakawaka James, Carl, John, Zachariah feature: dispensing labels Jan 29, 2025
Copy link

github-actions bot commented Jan 29, 2025

Bundle size difference

Comparing this PR to main

Old size New size Diff
5.28 MB 5.28 MB 1022 B (0.02%)

@GeronimoJohn GeronimoJohn marked this pull request as draft January 30, 2025 01:21
@GeronimoJohn
Copy link
Contributor Author

GeronimoJohn commented Jan 30, 2025

A conversation about how the input behaves are still in discussion within its ticket - converting to a draft until we've decided the way to go

@GeronimoJohn GeronimoJohn marked this pull request as ready for review January 30, 2025 22:25
@CarlosNZ CarlosNZ self-assigned this Feb 2, 2025
@CarlosNZ
Copy link
Contributor

CarlosNZ commented Feb 2, 2025

Looking good John, I haven't dug into the code yet, will do that next. Just wanted to comment on the layout/behaviour first. I see you've had some discussion on the issue, so I don't want to contradict everything that was agreed there. My main concern was minimising the effort required for the user.

In the vast majority of cases the Prescribed quantity will match the Issue quantity, so the user should only have to type the value once. But the way it is here, is initially focuses to the "Issue" quantity, and the "Prescribed" value stays at 0. Wouldn't it be better to put the inital focs on "Prescribed" so that the "Issue" value is filled automatically?

Following on from that, I was just chatting to James, and we thought that the "Tab" key from "Prescribed" should take you straight to Directions, as most of the time that's what you'll want.

Probably best just to have a quick IRL chat before changing anything though, so pop over and see me when you have a chance :)

Copy link
Contributor

@CarlosNZ CarlosNZ left a comment

Choose a reason for hiding this comment

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

Looking good, John.

I've left a few comments, mostly pretty minor stuff.

There's one bug I noticed that I can't quickly figure out what's happening:

  • Go into a prescription that already has lines (or create one), make sure there is a prescribed quantity and issue quantity for each.
  • Add a new item, then save
  • Now go back to the first item on the list and open the "Quantity" accordion -- the "Issue" quantity has been reset to 0! (If you refresh the page it'll come back).

Good luck tracking that one down :) j

Copy link
Contributor

@CarlosNZ CarlosNZ left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the fixes.

And as discussed IRL, the other issue I found is actually this one and so nothing to do with your changes.

Happy to merge this now πŸ‘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: dispensing Team Piwakawaka James, Carl, John, Zachariah
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dispensing: Prescribed Quantity
2 participants