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

Add shipping method name to orders detail report #3662

Merged
merged 1 commit into from
Apr 1, 2019

Conversation

SDBowen
Copy link
Contributor

@SDBowen SDBowen commented Mar 28, 2019

What? Why?

Adds a Shipping Method column to the Orders And Distributors report. This allows the users to see the the shipping detail of each order.

Closes #3446

Before:

Screenshot from 2019-03-27 19-39-00

After:

Screenshot from 2019-03-27 19-04-20

What should we test?

Generating a Orders and Distributors report will show the result of this PR.

Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

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

Nice, welcome to OFN Steven!
Thanks for your contribution!

 Added column allows users to see the shipping method of each order.
@SDBowen SDBowen force-pushed the add_shipping_method branch from b7b8810 to 0f7ef26 Compare March 28, 2019 22:10
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Congrats! You solved an issue, improved our code style and extended our test coverage. Awesome.

Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

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

awesome, thanks a lot!!!

@RachL RachL self-assigned this Apr 1, 2019
@RachL
Copy link
Contributor

RachL commented Apr 1, 2019

Shipping method is now shown in the order and distribution report.

image

The report section is working as usual. This is ready to go.

@RachL RachL removed the pr-staged-es label Apr 1, 2019
@luisramos0 luisramos0 merged commit d48d2a7 into openfoodfoundation:master Apr 1, 2019
@luisramos0
Copy link
Contributor

luisramos0 commented Apr 1, 2019

I have merged this PR exceptionally because I am curious if the contributors list gets updated. @SDBowen would be contributor number 93
I see that @amers185 has not been added after merging #3644 (although I see the commit in #3644 is from unknown user Usama.

@luisramos0
Copy link
Contributor

yeah, so @SDBowen has gone into the list of contributors but @amers185 has not, probably because the way @amers185 has configured git locally the commit got a different user that doesn't match the GH user.

@amers185
Copy link
Contributor

amers185 commented Apr 4, 2019

@luisramos0 Hello Luis! What can I do correctly on my end to go on the list of contributors? I was working on the branch created by @kevinchristianson perhaps that has something to do with it? I have also cloned openfoodnetwork but I had trouble bringing it up to date because of insufficient permissions. So I decided to work on the branch created by @kevinchristianson

@luisramos0
Copy link
Contributor

it's not a problem with the branch, it's the commit itself.
something to do with your configuration of git.
maybe just this: https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-linked-to-another-user

@amers185
Copy link
Contributor

amers185 commented Apr 4, 2019

@luisramos0 Thank you for pointing me to a solution. I have made the requisite changes. Perhaps, I should have mentioned earlier but my name is Usama so that is in fact me.

@luisramos0
Copy link
Contributor

yeah, I got that it was you. It's probably your local account. Good luck with that, let us know on slack if you need help.

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

Successfully merging this pull request may close these issues.

5 participants