Skip to content

Conversation

@mpvphd
Copy link

@mpvphd mpvphd commented Feb 19, 2025

  1. Manifest updated

  1. Default value of on_demand
    1. set to False

  1. Tests of on_demand
    1. Test Default Value of on_demand:
      • Ensures that the default value of the on_demand attribute on the product is False.
    2. Test Inheritance and Setting of on_demand:
      • Verifies that setting on_demand on a specific product instance correctly inherits and reflects its value when calling _stock_state_check_on_demand().
    3. Test Computation of Stock State When on_demand is True:
      • Confirms that the computed stock state is set to 'on_demand' regardless of the previous state,

  1. Updated view 'on demand' feature display in product.template view
    1. state_thershold not shown in view if on_demand is True

image

@ak-git-bot
Copy link

Hi @kevinkhao, @sebastienbeau, @legalsylvain,
some modules you are maintaining are being modified, check this out!

@mpvphd mpvphd changed the title [UPD] Updated 'on demand' feature display in product view [UPD] Updated 'on demand' feature Feb 19, 2025
manual_stock_state_threshold = fields.Float(digits="Stock Threshold")

on_demand = fields.Boolean(
default=False,
Copy link
Member

Choose a reason for hiding this comment

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

I think that by default we get False value for Boolean fields

Copy link
Author

@mpvphd mpvphd Feb 20, 2025

Choose a reason for hiding this comment

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

True !
in the end this is useless but harmless piece of code too ^^
thx


def test_06_state_on_demand_default_value(self):
"""Test default value of on_demand"""
self.assertFalse(self.product_threshold_on_product.on_demand)
Copy link
Member

Choose a reason for hiding this comment

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

useless test

"""Test on_demand Setting (Setting on a product unique template)"""
self.assertEqual(
self.product_threshold_on_product._stock_state_check_on_demand(),
self.product_threshold_on_product.on_demand,
Copy link
Member

Choose a reason for hiding this comment

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

useless

@hparfr
Copy link
Member

hparfr commented Mar 5, 2025

@mpvphd

Can you please fix the tests by

  • removing useless tests
  • adding a simple test case :
    • set stock level to x, set on_demand = true, set threshold = y (with x < y), expect state = on_demand
    • set stock level to x, set on_demand = true, set threshold = z (with z > y), expect stated= on_demand

@mpvphd
Copy link
Author

mpvphd commented Mar 5, 2025

@hparfr

  • please be more specific; indeed, the “usefulness” is a matter of point of view (To clarify this point, the test checks that no state overrides on_demand as soon as on_demand is true)
  • test_06_state_on_demand_computation test follows the existing logic, similar to test_05_state_out_of_stock, which does not depend on stock quantity properties (such as amount or threshold). Adding stock quantity-dependent checks would be unnecessary and inconsistent with the original test design.

@hparfr
Copy link
Member

hparfr commented Mar 5, 2025

test_06_state_on_demand_computation test follows the existing logic, similar to test_05_state_out_of_stock, which does not depend on stock quantity properties (such as amount or threshold). Adding stock quantity-dependent checks would be unnecessary and inconsistent with the original test design.

No.
I ask to write a test to clarify a behavior: what happens if there is stock for a product on_demand. As it is written now; the test 06 do not show this behavior.

@mpvphd mpvphd marked this pull request as draft March 5, 2025 15:04
@mourad-ehm mourad-ehm force-pushed the 14-Imp-prod_stock_state_on_demand branch from f99c6b0 to 3de0531 Compare March 24, 2025 14:48
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.

4 participants