-
Notifications
You must be signed in to change notification settings - Fork 16
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
6455 cancelled prescription status #6509
Conversation
Bundle size differenceComparing this PR to
|
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 good, just want to merge a couple of quick fixes before merging
@@ -34,6 +34,7 @@ const createStatusLog = (invoice: PrescriptionRowFragment) => { | |||
[InvoiceNodeStatus.Allocated]: null, | |||
[InvoiceNodeStatus.Shipped]: null, | |||
[InvoiceNodeStatus.Delivered]: null, | |||
[InvoiceNodeStatus.Cancelled]: null, |
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.
if (statusIdx >= 3) {
statusLog[InvoiceNodeStatus.Cancelled] = invoice.cancelledDatetime;
}
We might want something like this added, I think that will require chaging the PrescriptionRowFragment as well though.
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.
Added an issue follow up on this...
@@ -740,6 +740,7 @@ fn legacy_invoice_status(t: &InvoiceType, status: &InvoiceStatus) -> Option<Lega | |||
InvoiceStatus::Shipped => LegacyTransactStatus::Fn, | |||
InvoiceStatus::Delivered => LegacyTransactStatus::Fn, | |||
InvoiceStatus::Verified => LegacyTransactStatus::Fn, | |||
InvoiceStatus::Cancelled => LegacyTransactStatus::Fn, // TODO enable cancelled status to sync with mSupply https://github.com/msupply-foundation/open-msupply/issues/6495 |
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.
Thanks for these comments!
@@ -213,6 +214,7 @@ impl ActivityLogNodeType { | |||
from::DemographicIndicatorUpdated => to::DemographicIndicatorUpdated, | |||
from::DemographicProjectionCreated => to::DemographicProjectionCreated, | |||
from::DemographicProjectionUpdated => to::DemographicProjectionUpdated, | |||
&repository::ActivityLogType::InvoiceStatusCancelled | &repository::ActivityLogType::PrescriptionStatusCancelled => todo!() |
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.
We don't really want to have todo!
code in the develop branch, will push a fix for this, mapping to the right fields.
from::PrescriptionStatusCancelled => to::PrescriptionStatusCancelled,
from::InvoiceStatusCancelled => to::InvoiceStatusCancelled,
@@ -184,6 +186,7 @@ export const isInboundDisabled = (inbound: InboundRowFragment): boolean => { | |||
case InvoiceNodeStatus.Allocated: | |||
// Inbound shipments can be edited when having been delivered | |||
case InvoiceNodeStatus.Delivered: | |||
case InvoiceNodeStatus.Cancelled: |
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.
I think this needs to be in the disabled section, after Verified
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.
Thanks @zachariah-at-msupply good to merge!
Fixes #6455
π©π»βπ» What does this PR do?
Adds the cancelled status for invoices.
This means prescriptions can now have a cancelled status.
π Any notes for the reviewer?
π§ͺ Testing
π Documentation