-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update JSON menu representation in mutations #4524
Update JSON menu representation in mutations #4524
Conversation
Maybe this PR could include a fix for #4471? |
Should be fixed long time ago |
@dominik-zeglen the issue is not the dashboard, but the mutation not checking the user input 👍 |
40121ad
to
0b2761b
Compare
Here is the report for a801c56 (maarcingebala/saleor @ update-menu-json-in-api) No differences were found. (click me)
# api.benchmark checkout
test name left count right count duplicate count
------------------------------------ ----------- ----------- ---------------
add billing address to checkout 34 34 20
add shipping to checkout 7 7 0
checkout payment charge 14 14 0
complete checkout 6 6 0
create checkout 41 41 20
# api.benchmark homepage
test name left count right count duplicate count
------------------------------------ ----------- ----------- ---------------
retrieve main menu 5 5 0
retrieve product list 4 4 0
retrieve secondary menu 5 5 0
retrieve shop 2 2 0
# api.benchmark product
test name left count right count duplicate count
------------------------------------ ----------- ----------- ---------------
product details 13 13 3
# api.benchmark variant
test name left count right count duplicate count
------------------------------------ ----------- ----------- ---------------
retrieve variant list 9 9 2 |
Codecov Report
@@ Coverage Diff @@
## master #4524 +/- ##
==========================================
+ Coverage 90.64% 90.64% +<.01%
==========================================
Files 297 297
Lines 17437 17411 -26
Branches 1748 1738 -10
==========================================
- Hits 15805 15783 -22
Misses 1121 1121
+ Partials 511 507 -4
Continue to review full report at Codecov.
|
00ea60f
to
c78fdd2
Compare
@NyanKiyoshi I also fixed the issue #4471 that you've mentioned. |
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.
We might want to update dashboard-next as well before merging, I think it is breaking it, right?
Co-Authored-By: NyanKiyoshi <hello@vanille.bid>
@NyanKiyoshi What do you exactly mean by "updating dashboard-next"? I made fixes only in the backend. |
@maarcingebala nevermind. At the time of reviewing the |
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.
LGTM 👍
Storefront 1.0 uses
json_content
fields to render menus in its HTML templates (to optimize rendering and not fetch the whole structure from the database every time). Those fields were however not updated when using API mutations.This PR introduces the following changes:
update_menu
andupdate_menus
calls in mutations that touch menu itemssaleor.dashboard.menu.utils
tosaleor.menu.utils
as they're now used outside of dashboard 1.0 as well.Also fixes #4471
Screenshots
Pull Request Checklist