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

[MODORDERS-964] Update acq-models #806

Merged
merged 2 commits into from
Dec 19, 2023
Merged

[MODORDERS-964] Update acq-models #806

merged 2 commits into from
Dec 19, 2023

Conversation

alb3rtino
Copy link
Contributor

@alb3rtino alb3rtino commented Dec 14, 2023

Purpose

This PR updates acq-models to include the customFields attribute to purchase orders and purchase order lines. This is part of the custom fields integration. See MODORDERS-964.

Prerequisite

Related

@alb3rtino alb3rtino requested a review from a team December 14, 2023 20:56
@SerhiiNosko
Copy link
Contributor

@alb3rtino we also need to create PR with updating models for mod-gobi repository, because it also uses the same schemas

@alb3rtino
Copy link
Contributor Author

MODGOBI PR#220

@SerhiiNosko
Copy link
Contributor

SerhiiNosko commented Dec 15, 2023

checked that also need to create similar PRs to update references in mod-invoice, mod-invoice-storage
https://github.com/folio-org/mod-invoice/blob/master/descriptors/ModuleDescriptor-template.json#L574

@alb3rtino
Copy link
Contributor Author

Taking a closer look at this, as schemas for composite_po_line and composite_purchase_order have had additions, we might want to increase interface versions too, right?

order-lines -> 3.3
orders -> 12.1

@alb3rtino
Copy link
Contributor Author

I've updated to provided interface versions to

order-lines -> 3.3
orders -> 12.1

and the required interface versions to

orders-storage.po-lines -> 12.3
orders-storage.purchase-orders -> 8.1

This requires a merge of MODORDSTOR PR#374 first, so i've added the 'do not merge' tag.

@alb3rtino alb3rtino requested review from SerhiiNosko and a team December 18, 2023 10:57
Copy link

sonarcloud bot commented Dec 18, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@alb3rtino
Copy link
Contributor Author

@SerhiiNosko I've made updates to the interface versions. Could you please review them? I'm uncertain whether the updated required interfaces require a module major version increase or not.

@@ -4,7 +4,7 @@
"provides": [
{
"id": "orders",
"version": "12.0",
"version": "12.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

As I remember if we increase from 12.0 to 12.1 - Okapi will not throw an error, even if in mod-gobi required version is 12.0 https://github.com/folio-org/mod-gobi/blob/master/descriptors/ModuleDescriptor-template.json#L157
If we for example we increased from 12.0 to 13.0 and in mod-gobi remain 12.0 - Okapi will throw an error during enabling the module.
But you can check my words using karate tests(acquisition folder in folio-integration-test) or just locally.

Basically we not always follow the rule to update minor version when adding new field, but in case when you increased it also would be a good practice to increase in these modules as well:
https://github.com/folio-org/mod-invoice/blob/master/descriptors/ModuleDescriptor-template.json#L664
https://github.com/folio-org/mod-gobi/blob/master/descriptors/ModuleDescriptor-template.json#L157
https://github.com/folio-org/mod-ebsconet/blob/master/descriptors/ModuleDescriptor-template.json#L6

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically I am ok to keep major versions for example in mod-gobi 12.0 and in orders increase to 12.1. It should work

@alb3rtino alb3rtino merged commit 5fd3bec into master Dec 19, 2023
8 checks passed
@alb3rtino alb3rtino removed the DO NOT MERGE External dependencies. Do not merge yet. label Dec 19, 2023
@alb3rtino alb3rtino deleted the MODORDERS-964 branch December 19, 2023 12:25
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.

2 participants