Allow shipping category on variants #4739
Conversation
github-actions
bot
added
changelog:solidus_backend
tvdeyen
force-pushed
the
allow-shipping-category-on-variants
branch
from
72249ed to
3e7b80b
Compare
Codecov Report
@@ Coverage Diff @@
## master #4739 +/- ##
=======================================
Coverage 86.14% 86.14%
=======================================
Files 578 578
Lines 14654 14661 +7
=======================================
+ Hits 12623 12630 +7
Misses 2031 2031
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
core/db/migrate/20221123152807_add_shipping_category_to_spree_variants.rb
Show resolved
Hide resolved
tvdeyen
force-pushed
the
allow-shipping-category-on-variants
branch
from
3e7b80b to
8dd4c4c
Compare
|
@waiting-for-dev addressed |
|
@waiting-for-dev can you tell me why we have a bunch of unrelated CI fails? |
I can't 🙂 Re-running CI |
|
@tvdeyen, for some reason, tests are failing consistently. I don't understand why, as your changes don't seem related. However, #4741, submitted later, went through without problems. I thought that maybe it was because of the latter using Nebulab's pipeline, but I pushed another branch to this repo to run it with the solidusio pipeline, and tests also passed: https://github.com/solidusio/solidus/tree/testing_ci |
|
One thing that comes to mind, related to API, is that we probably need also to update the fields of the resources in the configuration file and the corresponding REST API documentation. Not sure how related this is to test failures, though. In the test results: |
tvdeyen
force-pushed
the
allow-shipping-category-on-variants
branch
from
8dd4c4c to
37e2e69
Compare
That one is funny. IMO a Rails Bug. It tries to create a foreign key from a table with a bigint as primary key to a column that is integer (or the other way round). Thats a common rails mysql issue. I now use the current active record migration schema version for the migration file instead of just 5.2 (the default for our migrations). Hope this will fix it. |
@kennyadsl could you explain what needs to be done case of the API documentation? The |
|
@tvdeyen right now we have the For the Does it make sense? |
@kennyadsl How do I add something to the API documentation? I couldn't find anything in the source, Wiki or README. Do I have to log into stoplight.io? |
|
@tvdeyen, look into the |
github-actions
bot
added
the
changelog:solidus_api
@waiting-for-dev thanks. I added the attribute. Hopefully into the correct place. I am new to OpenAPI schema, so bare with me. /cc @kennyadsl |
That's very hard to change, actually. We still couldn't find a better system, though. Thanks Thomas! |
|
@waiting-for-dev @kennyadsl the builds are still failing because of "Unable to locate package libvips". Any ideas? |
|
@tvdeyen, you can now rebase from master to fix CI 🙂 |
We must not reserve a complete grid column for the checkbox. Using the same look as on the product form.
tvdeyen
force-pushed
the
allow-shipping-category-on-variants
branch
from
bcd2527 to
0af8385
Compare
tvdeyen
force-pushed
the
allow-shipping-category-on-variants
branch
from
0af8385 to
45a0d3f
Compare
@waiting-for-dev I did. Thanks. Unfortunately it still fails: what seems to be very unrelated to my changes. Any idea? |
|
Only the mysql specs, though. |
|
Argh, still mysql does not like the foreign key @waiting-for-dev I already added the Rails version to the migration in the hope of fixing this issue with a more recent schema version, but this didn't turned out to work as well. Are you ok with removing the foreign key again? There is probably more work necessary to support foreign keys in Solidus with mysql. Something I do not want to tackle in this PR. |
Like with the tax category a variant could have a different shipping category. Think of a digital vs. physical variant of the same product.
tvdeyen
force-pushed
the
allow-shipping-category-on-variants
branch
from
45a0d3f to
470b90a
Compare
Absolutely. |
|
Fantastic work on this, @tvdeyen. This will benefit a number of our stores. 🙏🏻 |
Summary
Like with the tax category a variant could have a different shipping category.
Think of a digital vs. physical variant of the same product.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):