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

OP-1332 OP-1199 OP-1200 Pharmacy Inventory feature #2127

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

Conversation

mwithi
Copy link
Member

@mwithi mwithi commented Jan 28, 2025

See OP-1199 and OP-1200 (also addresses OP-1332).

mwithi and others added 30 commits July 25, 2024 13:54
OP-1303 | Make status more evident in edit inventory
* add logic to delete inventory

* add import

* extract inventory.getStatus()

* set status on UpperCamel case when create new Inventory

* Update src/main/java/org/isf/medicalinventory/gui/InventoryBrowser.java

* add missiong bundle

---------

Co-authored-by: ArnaudFonzam <[email protected]>
Co-authored-by: Alessandro Domanico <[email protected]>
OP-1302 | Make JTable sortable in edit inventory
* add validate action

* Update bundle/language_en.properties

* update logique and apply the new

* update  src/main/java/org/isf/medicalinventory/gui/InventoryEdit.java

* update bundle to add new message

* Rename one field

* Remove useless field and format code

* Remove deprecated field

* Fix exception after focus out on real qty

* Update one label

* update logic of validate Inventory

* update InventoryEdit

* update validate method

* update InventoryEdit

* update validate inventory'

* update Inventory Edit

* update InventoryEdit.java

* git fix error Index out of bound

* update Inventory

* apply suggestion

* fix error on update Cost of lot

* OP-1344: dialog method takes bundle key and not the final text (#2060)

* Add bundles

* Refactor validation method and change errors severity

* create new inventory during validation phase if we have new charge movement created after the creation date of inventory

---------

Co-authored-by: ArnaudFonzam <[email protected]>
Co-authored-by: David B Malkovsky <[email protected]>
Co-authored-by: Alessandro Domanico <[email protected]>
Co-authored-by: FOFOU FONZAM Gui Arnaud <[email protected]>
@ArnaudFofou
Copy link
Contributor

ArnaudFofou commented Jan 29, 2025

I'm not very sure that this appeareance is the best we can achieve though...

@dbmalkovsky @ArnaudFonzam do you think could be a good idea to just move the new two buttons inside the other three above? As normal buttons, they could be parametrized and so call a single "Browser" which differs only in the type of inventory (InventoryType) and few other things...

image

For me if we move those two buttons inside the other three it is not good we will have many intermediaire window when user just need to do and inventory.

please Have you see that ward Inventory button don't have shortcut. may be the problem is that the Shortcut used is already use on this window. I think we should update the menu item row related to ward inventory button to set the non used shortcut. WDYT?

@mwithi
Copy link
Member Author

mwithi commented Jan 29, 2025

please Have you see that ward Inventory button don't have shortcut. may be the problem is that the Shortcut used is already use on this window. I think we should update the menu item row related to ward inventory button to set the non used shortcut. WDYT?

Done, and also inverted item order, see informatici/openhospital-core#1474. Thanks!

@mwithi
Copy link
Member Author

mwithi commented Jan 29, 2025

For me if we move those two buttons inside the other three it is not good we will have many intermediaire window when user just need to do and inventory.

True, but in this way we have a lot of code duplication so when you want to change something you have to do it in two places. On the other hand, you have that the inventory is not a so frequent operation (generally yearly, hopefully quarterly, rarely monthly).
Anyway also other solution to avoid code duplication are warmly welcomed.

@mwithi mwithi changed the title OP-1199 OP-1200 Pharmacy Inventory feature OP-1332 OP-1199 OP-1200 Pharmacy Inventory feature Jan 31, 2025
MessageDialog.error(null, "angal.inventory.referencealreadyused.msg");
return;
}
inventory = new MedicalInventory();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
inventory = new MedicalInventory();
inventory = new MedicalInventory();
wardSelected = (Ward)wardComboBox.getSelectedItem();
inventory.setWard(wardSelected.getCode());

Copy link
Member Author

@mwithi mwithi Feb 3, 2025

Choose a reason for hiding this comment

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

the setWard method is already on the line 498, wardSelected is global and it should be already set at this point because ward selection is compulsory before to even start a new inventory, so in theory we could also remove the null check...

Have you experienced a bug?
Anyway we have also a global wardId, which seems just unnecessary if we already have wardSelected... I guess we can simplify a lot the logic here...

Copy link
Contributor

@ArnaudFofou ArnaudFofou Feb 3, 2025

Choose a reason for hiding this comment

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

yes I have the error after the inventory was created after checking I see that ward column was null even as we set the value before create its. I think the problem is because the value of selected ward is not set to the variable wardSelect and the value is null during creation. look the listener of wardComboBox you didn't set the value of the wardSelected but you used wardId, so you can use wardSelected instead of wardId on wardCombox Listener to avoid a lot of variable doing the same things.

Copy link
Member Author

Choose a reason for hiding this comment

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

As for now I've published a quick fix, please try.

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.

5 participants