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] adding new shipping category #5718

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

Conversation

loicginoux
Copy link
Contributor

Summary

This PR migrates the creation of new shipping categories to the new admin interface.
It follows the same kind of work done for Tax categories in #5674
There is no issue but a project item that I cannot move or comment, probably due to permission issues. https://github.com/orgs/solidusio/projects/12/views/1?filterQuery=xs&pane=issue&itemId=52590587

I also fixed searching shipping categories. This was a feature not present on old admin and by introducing new UI, and capability to search them, it was missing a ransack config to allow searching them by name.

Screen.Recording.2024-04-10.at.13.00.01.mov

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@github-actions github-actions bot added changelog:solidus_core Changes to the solidus_core gem changelog:solidus_admin labels Apr 10, 2024
@loicginoux loicginoux marked this pull request as ready for review April 10, 2024 11:19
@loicginoux loicginoux requested a review from a team as a code owner April 10, 2024 11:19
@kennyadsl
Copy link
Member

Thanks @loicginoux! I created #5720 to attach to this PR, thanks again!

@loicginoux loicginoux changed the title Feature/admin shipping category adding new shipping category in admin Apr 12, 2024
@loicginoux loicginoux changed the title adding new shipping category in admin [Admin] adding new shipping category Apr 12, 2024
@loicginoux
Copy link
Contributor Author

Not sure about the failing linter CI step. Should I look into it or is it something with the CI ? The error does not look like specific to this PR...

@kennyadsl
Copy link
Member

The error seems to be related to ESLint in the CI. I you want to take a look at it with another PR, you are more than welcome, thanks Loic!

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. ES lint error is handled in #5722

@loicginoux loicginoux force-pushed the feature/admin-shipping-category branch from e762007 to 5cc263b Compare April 17, 2024 13:13
Now that new admin allow for searching them, we had to allow name attrribute to be searchable via ransack
@loicginoux loicginoux force-pushed the feature/admin-shipping-category branch from 5cc263b to 55ec39e Compare April 17, 2024 13:13
@loicginoux
Copy link
Contributor Author

I just rebased branch from main after the linting CI error has been fixed in #5721. CI should be green here after that.

Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 97.29730% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 88.86%. Comparing base (3f6c58c) to head (55ec39e).

Files Patch % Lines
...rs/solidus_admin/shipping_categories_controller.rb 96.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5718      +/-   ##
==========================================
+ Coverage   88.84%   88.86%   +0.02%     
==========================================
  Files         704      705       +1     
  Lines       16757    16789      +32     
==========================================
+ Hits        14887    14919      +32     
  Misses       1870     1870              

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

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

Successfully merging this pull request may close these issues.

None yet

3 participants