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

Replaced rounded monetary amounts in emails with non-rounded ones #2392

Merged
merged 1 commit into from Oct 28, 2019

Conversation

Gbahdeyboh
Copy link
Contributor

No description provided.

@Gbahdeyboh Gbahdeyboh mentioned this pull request Aug 8, 2019
@Betree
Copy link
Member

Betree commented Aug 9, 2019

This is the fix for opencollective/opencollective#1773

@Gbahdeyboh It would be great to make changes on the same pull request next time, it is helpful for us to be able to see the full history on the same PR 😉

Can you take some screenshots of the emails to add in the description? Maildev can help you to do that: https://github.com/opencollective/opencollective-api/blob/master/docs/emails.md

@jeffin143
Copy link
Member

@Gbahdeyboh , are you still working on it :)

@Gbahdeyboh
Copy link
Contributor Author

@jeffin143 , i had some trouble visualizing it. For some reason, postgresql isn't working as expected on my laptop

@Betree
Copy link
Member

Betree commented Oct 17, 2019

@Gbahdeyboh we can't merge un-tested code. We have to make sure that this works and that it doesn't break anything.

We have some documentation that may help you to setup postgres, let me know if it helps:
https://github.com/opencollective/opencollective-api/blob/master/docs/postgres.md

@jeffin143
Copy link
Member

jeffin143 commented Oct 25, 2019

@Gbahdeyboh , Since it was difficult for you to setup the database, I pulled your branch and ran a test

@Betree - Here are the test for this branch

For approve Expense

Screenshot from 2019-10-25 21-14-24

And for Create Expense

Screenshot from 2019-10-25 21-07-51

I couldn't test it for Paid expense since I couldn't be a fiscal host :)

Copy link
Member

@jeffin143 jeffin143 left a comment

Choose a reason for hiding this comment

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

I ran the test (attached images above) and everything LGTM 💯

Green Signal from my side :)

Sorry, I approved it by mistake, just meant to leave it as a comment

@Gbahdeyboh
Copy link
Contributor Author

Thanks @jeffin143, I guess everything works as expected now?

@Betree
Copy link
Member

Betree commented Oct 28, 2019

Thanks for testing and providing screenshots @jeffin143 🎉

@Betree Betree merged commit 43fac8f into opencollective:master Oct 28, 2019
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.

None yet

3 participants