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

[SDP-343] Query fix for admin products list search by name #10310

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aplegatt
Copy link
Contributor

No description provided.

@aplegatt aplegatt marked this pull request as ready for review June 30, 2020 15:42
@aneskap
Copy link
Contributor

aneskap commented Sep 25, 2020

@aplegatt this is second PR in the SDP-343 task, do we need it? if yes could you maybe ask for another review & see why not all tests are passing? thank you :)

@aplegatt aplegatt closed this Oct 1, 2020
@aplegatt aplegatt reopened this Oct 1, 2020
@squash-labs
Copy link

squash-labs bot commented Oct 1, 2020

Manage this branch in Squash

Test this branch here: https://sdp-343-admin-products-list-fi-g1q5v.squash.io

Copy link
Contributor Author

@aplegatt aplegatt left a comment

Choose a reason for hiding this comment

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

@aneskap Yes, we do need it. Actually, all tests are passing, the error seems to be about not enough tests but the method I changed is already tested here: https://github.com/spree/spree/blob/master/core/spec/models/spree/product/scopes_spec.rb#L177.

@aplegatt aplegatt force-pushed the sdp-343-admin-products-list-fix branch from b9118c5 to f6c3d98 Compare January 5, 2021 16:14
@aneskap
Copy link
Contributor

aneskap commented Jan 12, 2021

@aplegatt I need an update on it - if this is ready for CR, could you post a request on our channel? or is it WIP still? :) thank you!

Copy link
Contributor Author

@aplegatt aplegatt left a comment

Choose a reason for hiding this comment

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

Ready to be merged.

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.

None yet

4 participants