Skip to content
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

[Admin][Product] fix properties edit action in admin panel part of the issue #5857 #5885

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Astr0surf3r
Copy link

@Astr0surf3r Astr0surf3r commented Oct 24, 2024

Summary

I have just downloaded the gem to fix the issue #5857 and I found in admin panel an error in Property Types (in your demo the error appear as well)

After I added a new one property and no issue came up clicking in the + Add new button of the /admin/properties page

new-properties

but after I saved it

properties-list

I couldn't edit it because an error appear

error-issue-solidus

so to fix this I add the show action in the Spree::Admin::PropertiesController and create an rspec file I hope this can help your great work

  • I have written a thorough PR description.

  • I have kept my commits small and atomic.

  • I have used clear, explanatory commit messages.

  • ✅ I have added automated tests to cover my changes.

  • 📸 I have attached screenshots to demo visual changes.

@Astr0surf3r Astr0surf3r requested a review from a team as a code owner October 24, 2024 20:31
@github-actions github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Oct 24, 2024
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Would you mind to update the link instead. We do not have show actions anywhere. We only ever have edit actions. By updating the link to the edit route it should be fixed

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.54%. Comparing base (b1a3796) to head (c8f23a7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5885   +/-   ##
=======================================
  Coverage   89.54%   89.54%           
=======================================
  Files         783      783           
  Lines       17994    17994           
=======================================
  Hits        16113    16113           
  Misses       1881     1881           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Astr0surf3r
Copy link
Author

Astr0surf3r commented Oct 30, 2024

Thanks. Would you mind to update the link instead. We do not have show actions anywhere. We only ever have edit actions. By updating the link to the edit route it should be fixed

@tvdeyen done! 6ce9444

@tvdeyen
Copy link
Member

tvdeyen commented Nov 4, 2024

@Astr0surf3r please use rebase to update your branch as mentioned in the contribution guidelines

@Astr0surf3r
Copy link
Author

Astr0surf3r commented Nov 4, 2024

c05d55a

@Astr0surf3r please use rebase to update your branch as mentioned in the contribution guidelines

@tvdeyen I hope now the PR is cleaner than before ... if need something else to do (I have read the guidelines) let me know please I really appreciate all your tips and messages to give a hand to your great project

NOTE: now the PR is out-of-date with the base branch for this reason launched an Update Branch command

Astr0surf3r and others added 7 commits November 4, 2024 12:24
This commit adds an introductory section telling the reader how the
migration process works.

It also adds a last section telling the reader to remove any ineligible
adjustments from their database.

Lastly, it corrects the location of Spree configuration from the
`solidus_promotions` initializer to the `spree.rb` initializer.
We don't want to include `solidus_promotions` as a dependency in
`solidus` yet, but we do want to release the gem, so it should appear in
the all the lists of Solidus sub-gems.
This code has been automatically generated by our 'Prepare release' GitHub
action.

The actual release is not part of the automation, and it still needs to be manually done by a maintainer.
Due to the top level constant in testing.rake the release
task was also trying to release frontend. Using a local
variable instead.
This code has been automatically generated by our 'Prepare post-release' GitHub
action.

Make sure that:

- [ ] The new release has been published, along with its tag. See https://github.com/solidusio/solidus/releases/tag/v4.4.0
- [ ] The corresponding patch branch exists. See https://github.com/solidusio/solidus/tree/v4.4
- [ ] The corresponding backport-v4.4 label exists. See https://github.com/solidusio/solidus/labels
@github-actions github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Nov 13, 2024
@github-actions github-actions bot removed the changelog:solidus_core Changes to the solidus_core gem label Nov 13, 2024
@tvdeyen
Copy link
Member

tvdeyen commented Nov 15, 2024

@Astr0surf3r please rebase with latest main branch and make one commit containing only your changes. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants