Skip to content

5535 Exclude requisition made from web entries in mSupply #5536

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

Merged
merged 14 commits into from
Apr 28, 2025

Conversation

raviSussol
Copy link
Collaborator

@raviSussol raviSussol commented Apr 7, 2025

Fixes #5535

Change summary

  • Removed wp and wf statuses from requisition schema and its related method to stop processing incoming requisitions made from web entries in the mSupply.
  • Fixed data linkage between Transaction & 'Requisiton` objects they have been broken from #5406 don't create blank supplier requisitions #5407

Testing

Steps to reproduce or otherwise test the changes of this PR:

  • Use the built apk and install on the tablet and use the mSupply installers to setup central server.
  • For developers the PR is here : https://github.com/msupply-foundation/msupply/pull/16410
    Brand new mobile setup
  • Setup any mobile site of Fiji moh with mSupply central server (i.e. use Fiji moh datafile)
  • Note copy Fiji moh mSupply data before you proceed below test cases - you'll need it for other variety of test cases
  • I used Samabula_pharm store's site
  • Initialize sync from mSupply to mobile
  • Login to the tablet and goto the supplier requisition and follow test 1
    Mobile upgrade setup
  • Use you Fiji moh mSupply data from second copy
  • Have a mobile apk version setup lower than v8.6.8
  • Setup sync with msupply central server with the above store's site
  • Login to the app and in the Supplier requisition you can now see some blank requisitions
  • Now upgrade the mobile app to v8.7.0
  • Login to the app after upgrade and do force sync
  • In the mSupply central server goto the scheduler dashboard window and force run scheduler_processMessages scheduler
  • Now in the mobile app again do force sync and follow test 2
  • Now export mobile data. You'll need admin pass thisisnepal123 enter that and click Export Data
  • Open that realm file in realm studio and follow test 3

Tests

  • Test 1

  • Should not display blank or having serial number 0 requisition

  • Also check the customer requisition - should not have any blank or having serial number 0 requisition

  • Please do regression tests related requisitions (both customer and supplier)

    • Setup 2 mobile apps and 1 oms site (i.e 1 oms central server with 1 oms remote) and 1 mSupply central server and 1 mSupply remote desktop site
    • Try creating some invoices (i.e from internal order or supplier requisition or request requisition -> response requisition or customer requisition -> customer invoice or outbound shipment (then via stock transfer) -> supplier invoice or inbound shipment
    • Do thorough tests on these workflow among OG mobile, oms site, msupply sites and see if there anything gets broken - should be working flawlessly
  • Test 2

  • All blank requisitions will be gone in the mobile and requisitions having wp status will also be gone from the mSupply

  • Test 3

  • In the Requisition object goto the linkedTransaction field where you can see linked transaction records in it

  • Similarly, in the Transaction object goto the linkedRequisition field where you can see linked requisition records in there

Copilot

This comment was marked as resolved.

@raviSussol raviSussol marked this pull request as draft April 8, 2025 03:23
no migration code need for cleaning blank requisition since we need to re-initialize mobile app from the sync server anyway so all of those blank requisitions will be handled there
@raviSussol raviSussol marked this pull request as ready for review April 9, 2025 06:28
@AnushaUp
Copy link

AnushaUp commented Apr 9, 2025

Testing

Tested in given APK
Steps to reproduce or otherwise test the changes of this PR:

  • Use the built apk and install on the tablet
  • Setup any mobile site of Fiji moh with mSupply central server (i.e. use Fiji moh datafile)
  • I used Samabula_pharm store's site
  • Initialize sync from mSupply to mobile
  • Login to the tablet and goto the supplier requisition and follow test 1

Tests

  • Should not display blank or having serial number 0 requisition
  • Also check the customer requisition - should not have any blank or having serial number 0 requisition
  • Please do regression tests related requisitions (both customer and supplier)
    • Setup 2 mobile apps and 1 oms site (i.e 1 oms central server with 1 oms remote) and 1 mSupply central server and 1 mSupply remote desktop site
    • Try creating some invoices (i.e from internal order or supplier requisition or request requisition -> response requisition or customer requisition -> customer invoice or outbound shipment (then via stock transfer) -> supplier invoice or inbound shipment
    • Do thorough tests on these workflow among OG mobile, oms site, msupply sites and see if there anything gets broken - should be working flawlessly

@raviSussol raviSussol added the Bug: production Bug was found or believed to be in a live release label Apr 14, 2025
Copy link
Contributor

@anildahalsussol anildahalsussol left a comment

Choose a reason for hiding this comment

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

Over all looks good to me.

@anildahalsussol
Copy link
Contributor

As seen while testing wp status requisition seems valid requisition with status fn and also can see linked response requisition and related requisition lines. So not sure should we deleted or if its valid then improve it looking at the response requisition?

@ujwalSussol
Copy link
Contributor

ujwalSussol commented Apr 23, 2025

update : was able to set up the Master mobile...
Now tomorrow will aim to build mobile via this PR.

I am using Node V14.21.3 ( think this is the best that we can do for now )

@ujwalSussol
Copy link
Contributor

@raviSussol : The testing I have done, looks positive...

Now I think what we need is a big Group test from @santoshkhanal12 and @AnushaUp and perhaps a few one of of us.

Copy link
Contributor

@ujwalSussol ujwalSussol left a comment

Choose a reason for hiding this comment

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

approved : But we do need to do a group test to make sure that daily activities are not messed up.

@raviSussol
Copy link
Collaborator Author

approved : But we do need to do a group test to make sure that daily activities are not messed up.

Yes that would be great to have other functional or regression tests before releasing the new version.

@@ -631,6 +631,7 @@ Transaction.schema = {
mode: { type: 'string', default: 'store' },
prescriber: { type: 'Prescriber', optional: true },
linkedRequisition: { type: 'Requisition', optional: true },
pendingRequisitionId: { type: 'string', optional: true, indexed: true },
Copy link
Contributor

@Chris-Petty Chris-Petty Jul 1, 2025

Choose a reason for hiding this comment

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

Bro got LLM'd? I can't find an indexed property in the realm docs and we've never set this anywhere else in the schema for this repo. That said, I'm surprised realm didn't throw a wobbly, either I haven't found the docs or realm is ignoring this?

Copy link
Collaborator Author

@raviSussol raviSussol Jul 2, 2025

Choose a reason for hiding this comment

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

That's a real property of realm js schema. https://www.mongodb.com/docs/realm-sdks/js/latest/types/Realm.PropertySchema.html. Agree that it hadn't been used before (maybe cause in the lower version before Mongodb owned Realm)? Although I was searching using LLMs for field indexing in the realm (to make realm query quicker for processing huge requisitions like some of our clients have Fiji, Png, CIV, etc) I had literally looked around their official docs to make sure that its not an LLM hallucination until I found that docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

to make realm query quicker for processing huge requisitions like some of our clients have Fiji, Png, CIV, etc

Are we having performance issues on mobile ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Heh yea I think I wasn't going deep enough couldn't find it in here which is where my initial googling took me, https://www.mongodb.com/docs/atlas/device-sdks/sdk/react-native/. Well I did end up first on some ancient version of the type docs you found like v0.7... thought it looked odd, the OG realm docs were much cleaner than any incarnation that MongoDB has created since 🙄 especially when they're trying to stuff this new Atlas thing down people's throats but have since deprecated it lol.

That said just found it in the proper docs! Not sure how I missed it before, though even if you search for it you get results mostly for their PHP Library first haha

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to make realm query quicker for processing huge requisitions like some of our clients have Fiji, Png, CIV, etc

Are we having performance issues on mobile ?

I haven't caught that so far but I basically used indexed property as an idea of relational database here in this case since pendingRequisitionId is a mirror representation (or foreign field) of Requisition.id (unlike an entire record object linked to a field) which is a primary key and has indexed property true by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: production Bug was found or believed to be in a live release tester: Anusha
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid Requisition appears to be created due to Finalised Supplier Invoice
6 participants