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

MBS-13719: Integrate new mail service into MusicBrainz Server #3363

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

JadedBlueEyes
Copy link
Contributor

Problem

MBS-13719: Integrate the new email service from my Google Summer of Code project with the MusicBrainz Server to send emails with the latest design!

Solution

  • I'm using the LWP instance from $self->c->lwp
  • Each request is built manually
  • I've added a new configuration option DBDefs->MAIL_SERVICE_BASE_URL
  • Request URLs are built off of that
  • JSON is serialised with JSON::XS qw( encode_json )

Testing

For each of the ones that I've added so far, I've tested the happy path manually

  • creating an account
  • reset a password
  • Send a message to a user with /user/<id>/contact, with different options set (particularly "Send a copy to my own email address")

Documenting

Draft progress

Language preferences

  • Allow users to specify a preferred language for mail
  • Send the user's preferred language to the email service

Emails integrated:

  • Email verification
  • Password reset
  • Editor messages
    • Send to self
  • Email in use
  • Lost username (email lookup)
  • Edit note
  • edit "No" vote
  • Subscriptions

Error handling

  • Die when failing to send an email

Tests

  • Remove old tests
  • Write automated tests for the integration

Further action

Once this is merged, the service will need to be configured and run in production. Updates to musicbrainz-docker might be needed.

'reply_to' => $EMAIL_NOREPLY_ADDRESS,
'params' => {
'to_name' => $to_name,
'verification_url' => "$verification_link",
Copy link
Member

Choose a reason for hiding this comment

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

If this is already a string, why stringify it again?

FWIW, at least locally the string does not seem to be empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That issue seems to have resolved itself. I believe it was because it was because bitmap and I were testing with an older version of the Mail service image 🤷. The reason for the two methods of stringification was effectively me throwing stuff at the wall to see what stuck - I can probably remove the first one now.

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.

2 participants