Skip to content

OBPIH-6757 Fix a bug related to 1:1 association between product suppl…#4904

Merged
awalkowiak merged 2 commits intodevelopfrom
OBPIH-6757-fix
Oct 23, 2024
Merged

OBPIH-6757 Fix a bug related to 1:1 association between product suppl…#4904
awalkowiak merged 2 commits intodevelopfrom
OBPIH-6757-fix

Conversation

@kchelstowski
Copy link
Collaborator

@kchelstowski kchelstowski commented Oct 21, 2024

…ier and product package + fix generating a source code

✨ Description of Change

A concise summary of what is being changed. Please provide enough context for reviewers to be able to understand the change and why it is necessary. If the issue/ticket already provides enough information, you can put "See ticket" as the description.

Link to GitHub issue or Jira ticket:

Description:
The fix for the main problem was adding a flush in product package save. Why it is needed is due to the fact of our complicated relationships in the product supplier domain - we have both 1:1 (defaultProductPackage) and 1:N (productPackages) relationships between product supplier and product package.
When attempting to save a productPackage without saving the productSupplier before, we would get an error that we try to use a "transient instance must be saved before current operation".
On the other hand, if we save the productSupplier before and then save the productPackage and assign it to the productSupplier everything looks great in the debugger - we can see that the productSupplier.defaultProductPackage is assigned properly as well as productSupplier.productPackages list contains the package.
The problem was that after the session was flushed and the transaction was commited, the productSupplier.defaultPackage was cleared (became null), whereas the list (productSupplier.productPackages) stayed untouched.
It's due to the queue that Hibernate generated for those operations as the transaction commit - when we call the save() without flush, the sql logs look like this:

Hibernate: 
    insert 
    into
        product_supplier
        (version, product_id, manufacturer_id, supplier_code, date_created, code, model_number, last_updated, default_product_package_id, active, standard_lead_time_days, tiered_pricing, supplier_id, manufacturer_code, comments, upc, brand_name, rating_type_code, manufacturer_name, name, product_code, supplier_name, contract_price_id, description, min_order_quantity, ndc, id) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
Hibernate: 
    insert 
    into
        product_package
        (version, product_id, date_created, gtin, last_updated, updated_by_id, uom_id, product_supplier_id, product_price_id, quantity, name, created_by_id, description, id) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)

so as you can see, we save the product supplier before the product package, so the default_product_package_id must be null at this point.
As for the 1:N bidirectional association, it is enough to store the foreign key in the product package table, so in the product_supplier_id property which is NOT null at this point (hence the list is not cleared, whereas the 1:1 is cleared).

When we call save(flush: true) though, the logs look like this:

Hibernate: 
    insert 
    into
        product_supplier
        (version, product_id, manufacturer_id, supplier_code, date_created, code, model_number, last_updated, default_product_package_id, active, standard_lead_time_days, tiered_pricing, supplier_id, manufacturer_code, comments, upc, brand_name, rating_type_code, manufacturer_name, name, product_code, supplier_name, contract_price_id, description, min_order_quantity, ndc, id) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
Hibernate: 
    insert 
    into
        product_package
        (version, product_id, date_created, gtin, last_updated, updated_by_id, uom_id, product_supplier_id, product_price_id, quantity, name, created_by_id, description, id) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
Hibernate: 
    update
        product_supplier 
    set
        version=?,
        product_id=?,
        manufacturer_id=?,
        supplier_code=?,
        date_created=?,
        code=?,
        model_number=?,
        last_updated=?,
        default_product_package_id=?,
        active=?,
        standard_lead_time_days=?,
        tiered_pricing=?,
        supplier_id=?,
        manufacturer_code=?,
        comments=?,
        upc=?,
        brand_name=?,
        rating_type_code=?,
        manufacturer_name=?,
        name=?,
        product_code=?,
        supplier_name=?,
        contract_price_id=?,
        description=?,
        min_order_quantity=?,
        ndc=? 
    where
        id=? 
        and version=?
Hibernate: 
    select
        this_.id as id1_80_0_,
        this_.version as version2_80_0_,
        this_.product_id as product_3_80_0_,
        this_.date_created as date_cre4_80_0_,
        this_.gtin as gtin5_80_0_,
        this_.last_updated as last_upd6_80_0_,
        this_.updated_by_id as updated_7_80_0_,
        this_.uom_id as uom_id8_80_0_,
        this_.product_supplier_id as product_9_80_0_,
        this_.product_price_id as product10_80_0_,
        this_.quantity as quantit11_80_0_,
        this_.name as name12_80_0_,
        this_.created_by_id as created13_80_0_,
        this_.description as descrip14_80_0_ 
    from
        product_package this_ 
    where
        (
            this_.uom_id=? 
            and this_.product_id=? 
            and this_.quantity=? 
            and this_.product_supplier_id=?
        ) limit ?
Hibernate: 
    update
        product_package 
    set
        version=?,
        product_id=?,
        date_created=?,
        gtin=?,
        last_updated=?,
        updated_by_id=?,
        uom_id=?,
        product_supplier_id=?,
        product_price_id=?,
        quantity=?,
        name=?,
        created_by_id=?,
        description=? 
    where
        id=? 
        and version=?

so the product supplier is updated with the existing product package, that has just been persisted - this didn't take place in the first version and this is why the productSupplier.defaultProductPackage was null after the transaction was commited.

📷 Screenshots & Recordings (optional)

If this PR contains a UI change, consider adding one or more screenshots here or link to a screen recording to help reviewers visualize the change. Otherwise, you can remove this section.

@kchelstowski
OBPIH-6757 Fix a bug related to 1:1 association between product suppl…
89334ab 
…ier and product package + fix generating a source code
@kchelstowski kchelstowski self-assigned this Oct 21, 2024
@github-actions github-actions bot added the domain: backend Changes or discussions relating to the backend server label Oct 21, 2024
awalkowiak
awalkowiak reviewed Oct 22, 2024
View reviewed changes
grails-app/services/org/pih/warehouse/data/ProductSupplierService.groovy
defaultProductPackage.quantity = quantity
productSupplier.addToProductPackages(defaultProductPackage)
productSupplier.defaultProductPackage = defaultProductPackage
productSupplier.save()
Copy link
Collaborator

@awalkowiak awalkowiak Oct 22, 2024

Choose a reason for hiding this comment

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

I think it would be worth adding a comment on why it is saved here and another on why the flush is needed two lines below.

Copy link
Collaborator

@awalkowiak awalkowiak Oct 22, 2024

Choose a reason for hiding this comment

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

(I know you described it in the description, but feels like it should be in the code)

@awalkowiak awalkowiak merged commit 403c229 into develop Oct 23, 2024
@awalkowiak awalkowiak deleted the OBPIH-6757-fix branch October 23, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@awalkowiak awalkowiak awalkowiak approved these changes

@ewaterman ewaterman Awaiting requested review from ewaterman

@drodzewicz drodzewicz Awaiting requested review from drodzewicz

@alannadolny alannadolny Awaiting requested review from alannadolny

Assignees

@kchelstowski kchelstowski

Labels

domain: backend Changes or discussions relating to the backend server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kchelstowski @awalkowiak