Make OC advanced settings work by permitting the extra parameter and make the OC edit page work with that option activated #5775
Conversation
luisramos0
force-pushed
the
oc_inv_bug
branch
2 times, most recently
from
d5352c1
to
c1886ab
Compare
…product queries and it's just not needed there. The only other use of this product's scope visible_for is the enterprise serializer so we add the select to it.
…r counting products
…om the coordinator inventory are renderer
luisramos0
changed the title
| products = renderer.exchange_products(false, exchange.receiver) | ||
|
|
||
| expect(products).to eq [exchange.variants.first.product] | ||
| end |
There was a problem hiding this comment.
this test blows up in master just like #5773
|
I missed that this fixes the steps to create as well and ended up creating #5802 for that bug. So that gives you a 2 issue PR @luisramos0 if you want it. |
|
ah, yes, that's fixed here as well (it was a very common rails 4 upgrade problem: incomplete list of permitted "strong" params). Thanks Steve. |
Apply PR #5775 to branch v3.1.2 to make v3.1.3 patch
|
Hi @luisramos0 , Thank you for your help reproducing this issue! Setting the an OC to "Coordinators Inventory only" breaks the Outgoing Products page. Staging this PR solves this issue. Several tests were done, in this regard, please see my notes in the release-draft testing here. I think this PR could be merged and the patch deployed, and two separate issues open. Awaiting feedback on this. |
|
I left a comment on the release issue. Let's merge this and create new issues if needed 👍 |
What? Why?
Closes #5773 & #5802
Setting the filter in the OC to products from coordinator inventory only was broken due to strong params permissions. That is fixed in this PR.
But the main issue 5773 was when the filter products from coordinator inventory only was active. For some reason in rails 4 the SQL query generation was going wrong and the list of fields in the select part of the SQL included the select from both the visible_for scope in product (which had a distinct in it) and the fields from the included table variants. In this PR I just remove the select from the visible_for scope as it is not needed for this exchange renderer.
What should we test?
I can see the problem with the live data on my local machine and I confirm the PR fixes the problem.
For this reason I havent tried to replicate the problem with other data/OC. I did write a auto test that validates the bug so I believe it as simple as activating the "inventory only" option and add some variants to the exchanges.
We should validate the OC page features around adding and removing different types of variants to the exchanges. This combined with activating the "inventory only" setting and validating it works as appropriate: only variants from the coordinators inventory should show in the page to be added to the OC.
Release notes
Changelog Category: Fixed
Fixed a bug in the Order Cycles edit page when the option "coordinator inventory only" was activated.