Stabilise shopping specs and open them for change #5667
Conversation
| # | ||
| # The auto-submit on these specific form elements (add to cart) now has a small built-in | ||
| # waiting period before submitting the data... | ||
| sleep 0.6 |
There was a problem hiding this comment.
I think this should be a method in itself as it was in the shopping_spec, that way the multiline comment becomes a method comment.
There was a problem hiding this comment.
Yes, I agree. I'll leave it in this case though because the mobile code will remove it again. It won't be needed.
| # | ||
| # This is temporary code. The duplication will be removed by the mobile | ||
| # product listings feature. This has been backported to avoid merge | ||
| # conflicts and to make the current build more stable. |
There was a problem hiding this comment.
This type of comment should never land on the codebase. The fact that is temporary doesn't need to be there. I'd remove this comment from the code and add it as a comment on the PR.
There was a problem hiding this comment.
My reason for the comment was so that if someone sees the code later that they don't start refactoring it. Somebody may come across the code without seeing the commit. I'll leave it for now. It will be removed soon anyway.
| accept_alert 'Insufficient stock available, only 5 remaining' do | ||
| fill_in "variants[#{variant.id}]", with: '10' | ||
| end | ||
| click_add_to_cart variant, 5 |
There was a problem hiding this comment.
Why did the behaviour change? Is there not an alert anymore? why?
There was a problem hiding this comment.
Oh, well spotted! That's a mistake. In the new mobile UX it won't be possible to click on the button once the stock limit is reached. But this spec change should happen then, not now. I will change it.
The way we add items to the cart will change. Encapsulating that code in a common place will make the mobile ux work clearer and avoid merge conflicts. The waiting for background requests has also been improved and made more consistent which should make these specs more reliable.
The spec is not supposed to test the navigation to the shop. Going directly to the shop reduces the test execution time by 7%.
|
Merge ready 😊 |
What? Why?
Preparing for #4598 and maybe solving #4957.
What should we test?
No test, only specs are affected.
Release notes
Preparations for more mobile friendly shop fronts.
Changelog Category: Changed