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

chore: update handlebars to version 4.7.1 #3693

Merged
merged 1 commit into from
Apr 23, 2020
Merged

chore: update handlebars to version 4.7.1 #3693

merged 1 commit into from
Apr 23, 2020

Conversation

bolariin
Copy link
Contributor

Resolves opencollective/opencollective#2802

I made use of info in getterMethods of the model as it was already used within the codebase. From what I could see the errors came from passing the sequelize instances directly as data.

@bolariin bolariin added the WIP label Apr 10, 2020
@bolariin bolariin marked this pull request as draft April 10, 2020 21:39
@bolariin bolariin marked this pull request as ready for review April 11, 2020 03:31
@bolariin bolariin removed the WIP label Apr 11, 2020
@znarf
Copy link
Member

znarf commented Apr 17, 2020

@Betree could you have a look at this?

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

I think it's a good way to solve this issue. We should carefully inspect the important emails to make sure there's nothing missing.

@@ -198,6 +198,7 @@ export default (Sequelize, DataTypes) => {
emailWaitingForValidation: this.emailWaitingForValidation,
createdAt: this.createdAt,
updatedAt: this.updatedAt,
emailConfirmationToken: this.emailConfirmationToken,
Copy link
Member

Choose a reason for hiding this comment

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

This is a private information that should never be shared, it should not appear in info.

Because there's only one email relying on that, I would suggest passing it manually where it's needed.

@bolariin bolariin force-pushed the update-handlebars branch 2 times, most recently from f5f2752 to 187a143 Compare April 18, 2020 01:47
@Betree
Copy link
Member

Betree commented Apr 20, 2020

@bolariin changes are good. I still need to test the emails, I'll try to do it today.

@Betree
Copy link
Member

Betree commented Apr 23, 2020

Sorry for the late update, busy week! Testing that now.

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

I tested a bunch of emails, including:

  • Sign in
  • Reset password
  • New member
  • Gift cards
  • New expense

Everything seems to work properly. Nice job @bolariin!

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.

Update handlebars to version 4.7.1
3 participants