Skip to content

Conversation

@sabrinaRMartin
Copy link

@sabrinaRMartin sabrinaRMartin commented Dec 16, 2025

Migration from version 15.0 to 18.0

This module depends on PR:

@moduon @Shide MT-13047

https://www.loom.com/share/2425212b1eaf4fd7955c72c468ef791f

Copy link

@Andrii9090 Andrii9090 left a comment

Choose a reason for hiding this comment

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

@sabrinaRMartin good job! Code&funcional review 👍

Test for partner:
Image
Image

Test for product:
Image

Image

Copy link
Contributor

@BhaveshHeliconia BhaveshHeliconia left a comment

Choose a reason for hiding this comment

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

Functional review LGTM!

Copy link
Contributor

@Shide Shide left a comment

Choose a reason for hiding this comment

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

Code review

@sabrinaRMartin sabrinaRMartin force-pushed the 18.0-mig-purchase_warn_option branch from 3081174 to 25a8d00 Compare January 8, 2026 09:03
Copy link

@Gelojr Gelojr left a comment

Choose a reason for hiding this comment

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

Congratulations on the work done @sabrinaRMartin. However, I think a review is necessary.
I have performed the following functional tests.

  • Test 1: OK — Partner warning option (Warning) correctly fills and saves the warning message from the selected Warn Option.
  • Test 2: OK — Partner warning option (Block) correctly fills and saves the blocking message from the selected Warn Option.
  • Test 3: NOT OK — Changing the Partner warning type after selecting a Warn Option keeps the previous message content and saves an inconsistent configuration.

Reproduction steps for Test 3 (NOT OK):

  1. Create two Warn Options for partner purchase warnings: one with allowed warning type = Warning, and another with allowed warning type = Block.
image
  1. Go to a Vendor (Partner) and set “Warning on the Purchase Order” = Warning.
  2. Select the Warn Option of type Warning and confirm the message fields are filled with the Warning content.
  3. Change “Warning on the Purchase Order” from Warning to Blocking Message.
  4. Observe that the message fields below are not updated and still show the previous Warning content.
image
  1. Save the vendor.

Observed behavior: The vendor ends up with “Blocking Message” selected, but the stored message text remains the one from the Warning-type option (inconsistent: Block type with Warning content).

Expected behavior: When switching the warning type (Warning ↔ Block), the message should be cleared or updated to match a Warn Option compatible with the new type, preventing saving a mismatched configuration.

@sabrinaRMartin sabrinaRMartin force-pushed the 18.0-mig-purchase_warn_option branch from 25a8d00 to 5f6701c Compare January 9, 2026 15:55
@Shide
Copy link
Contributor

Shide commented Jan 10, 2026

Congratulations on the work done @sabrinaRMartin. However, I think a review is necessary. I have performed the following functional tests.

  • Test 1: OK — Partner warning option (Warning) correctly fills and saves the warning message from the selected Warn Option.
  • Test 2: OK — Partner warning option (Block) correctly fills and saves the blocking message from the selected Warn Option.
  • Test 3: NOT OK — Changing the Partner warning type after selecting a Warn Option keeps the previous message content and saves an inconsistent configuration.

Reproduction steps for Test 3 (NOT OK):

  1. Create two Warn Options for partner purchase warnings: one with allowed warning type = Warning, and another with allowed warning type = Block.
image 2. Go to a Vendor (Partner) and set “Warning on the Purchase Order” = Warning. 3. Select the Warn Option of type Warning and confirm the message fields are filled with the Warning content. 4. Change “Warning on the Purchase Order” from Warning to Blocking Message. 5. Observe that the message fields below are not updated and still show the previous Warning content. image 6. Save the vendor.

Observed behavior: The vendor ends up with “Blocking Message” selected, but the stored message text remains the one from the Warning-type option (inconsistent: Block type with Warning content).

Expected behavior: When switching the warning type (Warning ↔ Block), the message should be cleared or updated to match a Warn Option compatible with the new type, preventing saving a mismatched configuration.

Not removing the message is intended.
This module only copies the text to the field that shows warning/blocks.
This review should be OK

Copy link

@Gelojr Gelojr left a comment

Choose a reason for hiding this comment

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

Congratulations on the great work.
I have performed the following functional tests.

  • Test 1: OK — Vendor: selecting a Warning-type Warn Option correctly fills the purchase warning message.
  • Test 2: OK — Vendor: selecting a Block-type Warn Option correctly fills the blocking message.
  • Test 3: OK — Vendor: the Warn Option selector is constrained by the selected warning type (it only allows Warning options when “Warning” is selected, and only Block options when “Blocking Message” is selected; it does not allow selecting a Warning message while “Blocking Message” is set, nor the other way around, and it also excludes product-specific options).
  • Test 4: OK — Product: selecting a Warning-type Warn Option correctly fills the purchase line warning message.
  • Test 5: OK — Product: generic Warn Options (no specific type) can be selected and are applied correctly.
  • Test 6: OK — When warning is set to no-message, the Warn Option selector is hidden.
  • Test 7: OK — Purchase Order: vendor warnings/blocks use the message coming from the selected Warn Option.
  • Test 8: OK — Purchase Order: product warnings use the message coming from the selected Warn Option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants