Upgrade email functionality in Oppia to have email attachments #21380

Nik-09 · 2024-12-05T07:14:37Z

Overview

Currently, our emailing functionality does not allow for sending emails with attachments. This PR will extend the current functionality to include such a provision.

Essential Checklist

Please follow the instructions for making a code change.

I have linked the issue that this PR fixes in the "Development" section of the sidebar.
I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
I have written tests for my code.
The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Testing doc (for PRs with Beam jobs that modify production server data)

A testing doc has been written: ... (ADD LINK) ...
(To be confirmed by the server admin) All jobs in this PR have been verified on a live server, and the PR is safe to merge.

Proof that changes are correct

I have substituted my Mailgun credentials in a unit test and demonstrated the actual mail-sending functionality with the attachment in the video below.

mail.mp4

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

oppiabot · 2024-12-05T07:14:40Z

Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it modifies files in jobs or platform folders.
Also @Nik-09, please make sure to fill in the server jobs form for the new job or feature to be tested on the backup server. This PR can be merged only after the test run is successful. Please refer to this guide for details.
Thanks!

oppiabot · 2024-12-05T07:14:50Z

Hi @Nik-09 please assign the required reviewer(s) for this PR. Thanks!

Nik-09 · 2024-12-06T06:36:50Z

core/platform/email/mailgun_email_services.py

@@ -95,16 +100,17 @@ def send_email_to_recipients(
    # To send bulk emails we pass list of recipients in 'to' paarameter of
    # post data. Maximum limit of recipients per request is 1000.
    # For more detail check following link:
-    # https://documentation.mailgun.com/user_manual.html#batch-sending
+    # https://documentation.mailgun.com/docs/mailgun/user-manual/


Updated documentation link.

Nik-09 · 2024-12-06T06:37:38Z

core/platform/email/mailgun_email_services.py

    recipient_email_lists = [
        recipient_emails[i:i + 1000]
        for i in range(0, len(recipient_emails), 1000)]
    for email_list in recipient_email_lists:
        data = {
            'from': sender_email,
-            'subject': subject.encode('utf-8'),


No need to encode manually because the request library will handle the encoding intrinsically.

Nik-09 · 2024-12-06T06:41:45Z

core/platform/email/mailgun_email_services.py

-        # also the docs recommend this approach:
-        # https://docs.python.org/3.7/library/urllib.request.html#urllib-examples
-        encoded_url = urllib.parse.urlencode(data).encode('ascii')
-        req = urllib.request.Request(server, encoded_url, header)


Updated the implementation from urllib to the requests library as it simplifies attaching files in multipart/form-data and provides built-in support for this format.

oppiabot · 2024-12-09T15:07:00Z

Hi @Nik-09. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

oppiabot · 2024-12-09T15:07:02Z

Hi @Nik-09. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

Nik-09

Hi @lkbhitesh07 @kevintab95 @U8NWXD PTAL once you get time. Thank you!

kevintab95

Thanks @Nik-09! One question, how do we test this?

oppiabot · 2024-12-16T05:45:42Z

Hi @Nik-09, it looks like some changes were requested on this pull request by @kevintab95. PTAL. Thanks!

Nik-09 · 2024-12-17T07:22:27Z

Thanks @Nik-09! One question, how do we test this?

Hi @kevintab95 I have substituted my Mailgun credentials in a unit test and demonstrated the actual mail-sending functionality with the attachment in the video added in the PR description, PTAL.

kevintab95

Thanks @Nik-09 for explaining how you tested this! LGTM

oppiabot · 2024-12-26T07:53:24Z

Hi @Nik-09, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 4 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 4 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

lkbhitesh07

Sorry for the delayed review @Nik-09, LGTM!

oppiabot · 2024-12-28T15:56:16Z

Hi @Nik-09, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

Nik-09 added 2 commits December 2, 2024 14:39

Updates email method to have attachments

5d63a5f

Fixes backend tests

f2b3b0b

oppiabot bot assigned DubeySandeep, kevintab95 and vojtechjelinek Dec 5, 2024

oppiabot bot added the PR: Affects datastore layer Labels to indicate a PR that changes the datastore layer. label Dec 5, 2024

oppiabot bot assigned Nik-09 Dec 5, 2024

Nik-09 unassigned vojtechjelinek, kevintab95 and DubeySandeep Dec 6, 2024

Nik-09 commented Dec 6, 2024

View reviewed changes

Fixes lint issues

09026c5

Nik-09 mentioned this pull request Dec 9, 2024

[Feature Request]: Auto Generate Voiceovers using Azure Speech Synthesis service. #16837

Open

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Dec 9, 2024

Nik-09 added 2 commits December 10, 2024 00:32

Fixes merge conflict

47c7ac8

Fixes lint test

94ee84c

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Dec 10, 2024

Nik-09 marked this pull request as ready for review December 10, 2024 07:51

Nik-09 requested a review from a team as a code owner December 10, 2024 07:51

Nik-09 requested a review from lkbhitesh07 December 10, 2024 07:51

Fixes backend tests

4e4e0eb

Nik-09 requested a review from a team as a code owner December 10, 2024 19:34

Nik-09 requested a review from U8NWXD December 10, 2024 19:34

Fixes mypy & backend tests

a05759b

Nik-09 requested a review from a team as a code owner December 12, 2024 04:07

Nik-09 requested a review from kevintab95 December 12, 2024 04:07

Nik-09 commented Dec 12, 2024

View reviewed changes

Nik-09 assigned kevintab95, U8NWXD and lkbhitesh07 and unassigned Nik-09 Dec 12, 2024

Merge branch 'develop' into upgrade_email_functionality

3c0d86c

U8NWXD approved these changes Dec 13, 2024

View reviewed changes

U8NWXD removed their assignment Dec 13, 2024

kevintab95 requested changes Dec 16, 2024

View reviewed changes

kevintab95 removed their assignment Dec 16, 2024

oppiabot bot assigned Nik-09 Dec 16, 2024

Nik-09 assigned kevintab95 and unassigned Nik-09 Dec 17, 2024

kevintab95 approved these changes Dec 19, 2024

View reviewed changes

kevintab95 removed their assignment Dec 19, 2024

oppiabot bot added the stale label Dec 26, 2024

Removes unnecessary try-except block

8f61992

oppiabot bot removed the stale label Dec 28, 2024

lkbhitesh07 approved these changes Dec 28, 2024

View reviewed changes

lkbhitesh07 assigned Nik-09 and unassigned lkbhitesh07 Dec 28, 2024

oppiabot bot added the PR: LGTM label Dec 28, 2024

Nik-09 added this pull request to the merge queue Dec 30, 2024

Merged via the queue into oppia:develop with commit c03e8c1 Dec 30, 2024
130 checks passed

Nik-09 deleted the upgrade_email_functionality branch December 30, 2024 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade email functionality in Oppia to have email attachments #21380

Upgrade email functionality in Oppia to have email attachments #21380

Nik-09 commented Dec 5, 2024 •

edited

Loading

oppiabot bot commented Dec 5, 2024

oppiabot bot commented Dec 5, 2024

Nik-09 Dec 6, 2024

Nik-09 Dec 6, 2024 •

edited

Loading

Nik-09 Dec 6, 2024

oppiabot bot commented Dec 9, 2024

oppiabot bot commented Dec 9, 2024

Nik-09 left a comment

kevintab95 left a comment

oppiabot bot commented Dec 16, 2024

Nik-09 commented Dec 17, 2024

kevintab95 left a comment

oppiabot bot commented Dec 26, 2024

lkbhitesh07 left a comment

oppiabot bot commented Dec 28, 2024

Upgrade email functionality in Oppia to have email attachments #21380

Upgrade email functionality in Oppia to have email attachments #21380

Conversation

Nik-09 commented Dec 5, 2024 • edited Loading

Overview

Essential Checklist

Testing doc (for PRs with Beam jobs that modify production server data)

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

oppiabot bot commented Dec 5, 2024

oppiabot bot commented Dec 5, 2024

Nik-09 Dec 6, 2024

Choose a reason for hiding this comment

Nik-09 Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Nik-09 Dec 6, 2024

Choose a reason for hiding this comment

oppiabot bot commented Dec 9, 2024

oppiabot bot commented Dec 9, 2024

Nik-09 left a comment

Choose a reason for hiding this comment

kevintab95 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Dec 16, 2024

Nik-09 commented Dec 17, 2024

kevintab95 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Dec 26, 2024

lkbhitesh07 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Dec 28, 2024

Nik-09 commented Dec 5, 2024 •

edited

Loading

Nik-09 Dec 6, 2024 •

edited

Loading