-
Notifications
You must be signed in to change notification settings - Fork 140
[ENH] 10.0 raise warnings #84
base: 10.0
Are you sure you want to change the base?
[ENH] 10.0 raise warnings #84
Conversation
93bc2d5
to
e4da031
Compare
Modified for new refactored domain validations.
e4da031
to
0bb2c03
Compare
Codecov Report
@@ Coverage Diff @@
## 10.0 #84 +/- ##
==========================================
- Coverage 61.11% 60.99% -0.12%
==========================================
Files 14 14
Lines 1368 1382 +14
==========================================
+ Hits 836 843 +7
- Misses 532 539 +7
Continue to review full report at Codecov.
|
@@ -436,24 +438,36 @@ def values_available(self, attr_val_ids, sel_val_ids): | |||
avail = self.validate_domains_against_sels(domains, sel_val_ids) | |||
if avail: | |||
avail_val_ids.append(attr_val_id) | |||
|
|||
elif do_raise: | |||
if len(config_lines) == 1 and config_lines[0].rule_description: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has pretty limited applicability, maybe we can find a better way to raise all the errors one by one as they are fixed or drop it I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our use customer's use case is:
They configures the product on a single view, with around 15 options. Some of the rules and dependencies were too complicated to reflect in readonly's on the view (the other PR will fix a lot of that). Then they clicked submit and they just got a "configuration is invalid - please check all the steps" error, which did not help them focus on the problem.
The real benefit of this approach is where a configurable product has been defined badly. A value on step 3 may make a value on step 1 invalid! This is a bad setup. But, the rendering of the page cannot foresee this (and nor should it have to). If they save the configuration after step 3 in this scenario, it just says "please check all your steps", but with a focussed error message, they will see ("Value X should not be chosen when the product is Blue or Red")....
I did a proper re-base to remove all the "merge" commits.