-
-
Notifications
You must be signed in to change notification settings - Fork 717
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
Customer names in reports #5769
Customer names in reports #5769
Conversation
79cb824
to
c3ba056
Compare
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.
All right!
It's a new feature, I think it's better if we get some screenshots of the new config item and of a report with and without names.
I left some comments to refactor some of the code.
The only reason why I am not approving the PR yet is that I think it should be prefers_ not preferred_?
app/views/spree/admin/reports/_customer_names_message.html.haml
Outdated
Show resolved
Hide resolved
c3ba056
to
9f45a18
Compare
9f45a18
to
2f5efc8
Compare
I wanted to do a lot more here, but was told to do the minimum to cover the epic. I've updated and refactored a lot more 👍 |
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.
awesome, thanks Matt. New service 👍
I think I'd still go with prefers_ but now I see it's only for readability.
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.
I like the extraction into a service. 🌷
Hey @Matt-Yorkley , Thanks for this new feature! Had a look at the different scenarios in your test notes, and as well on the feature requests from @RachL . Before staging this PR
As a hub/enterprise manager C:
As a customer D:
As Producer A:
After staging this PR As a hub/enterprise manager C:
As Producer A:
As a hub/enterprise manager C:
As Producer A:
Awesome work! Thank you @RachL for inception and @Matt-Yorkley for implementing 💪 |
What? Why?
Closes #5054, #5097
What should we test?
Enterprises can set an "allow customer names" preference in admin enterprise edit page, under "Shop Preferences". The default selection is "disabled".
The following reports need to be tested:
The test conditions are as follows:
Release notes
Added option for distributors to allow their suppliers to see customer names in reports, for improved packing