feat: send individuals emails with pluggable component #6

johnvente · 2023-12-18T13:34:40Z

Description

This feature will allow sending emails to expected emails of students in the course using pluggable component made here

NOTE: For scheduled emails to be sent, we would need to modify this endpoint /api/instructor_task/v1/schedules/course-v1:edX+DemoX+Demo_Course/bulk_email/?page=1 to include "extra_targets" in order to view the previously added emails.

DEMO

Pluggin loaded

How to test it

If you are using devstack

Go to workspace and clone this branch

cd ~/workspace && git remote add fork [email protected]:eduNEXT/frontend-app-communications.git

git fetch fork jv/feat-send-individual-email-learners && git checkout jv/feat-send-individual-email-learners

Go to the mfe repository, use node version in the repo and install dependencies

cd ~/workspace/frontend-app-communications && nvm use && npm install

inside frontend-app-communications run

npm run start

Go to http://localhost:1984/courses/course-v1:edX+DemoX+Demo_Course/bulk_email Or
http://localhost:1984/courses/{{id_course}}/bulk_email
In the input with this label "Add individual learner" search an student with email/name/username and click on the user that you wish
Fill out the fields Subject and Body

codecov-commenter · 2023-12-18T14:04:04Z

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (cd64f47) 83.42% compared to head (ced6b97) 81.65%.

Files	Patch %	Lines
.../bulk-email-tool/bulk-email-form/BulkEmailForm.jsx	57.14%	9 Missing ⚠️
...l-form/bulk-email-recipient/BulkEmailRecipient.jsx	33.33%	2 Missing ⚠️

Additional details and impacted files

@@                       Coverage Diff                       @@
##           jv/pluggable-component-slot       #6      +/-   ##
===============================================================
- Coverage                        83.42%   81.65%   -1.77%     
===============================================================
  Files                               48       48              
  Lines                              718      747      +29     
  Branches                           140      144       +4     
===============================================================
+ Hits                               599      610      +11     
- Misses                             119      137      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mariajgrimaldi

My main question is about the changes in the MFE not related to the component slots, do we need them here (right in the MFE) so the plugin works? or can they be moved to the plugin?

mariajgrimaldi · 2023-12-18T16:19:25Z

plugins/communications-app/IndividualEmails/api.js

+import { logError } from '@edx/frontend-platform/logging';
+
+export async function getLearnersEmailInstructorTask(courseId, search) {
+  const endpointUrl = `${getConfig().LMS_BASE_URL}/platform-plugin-communications/${courseId}/api/search_learners?query=${search}&page=1&page_size=10`;


why is the page and page_size fixed? shouldn't it be parametrized?

mariajgrimaldi · 2023-12-18T16:30:12Z

plugins/communications-app/IndividualEmails/index.jsx

+      // eslint-disable-next-line no-console
+      console.error('error autocomplete input', error.message);


why a console error instead of a standard logError?

mariajgrimaldi · 2023-12-18T16:34:42Z

plugins/communications-app/IndividualEmails/index.jsx

+        renderMenuItemChildren={({ name, email }) => (
+          <span data-testid="autocomplete-email-option">{name ? `${name} -` : name} {email}</span>


The search query can also be the username. Would the name/email combination appear even though I'm looking for the student by their username? wouldn't that be confusing?

I can add the username as well, no problem. For me is not confusing due to the user can see the name/email of the student in the list an selected that one

mariajgrimaldi · 2023-12-18T16:39:09Z

src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx

+    const emailsIndividualLearners = emailLearnersList.map(({ email }) => email);
+    const extraTargets = { emails: emailsIndividualLearners };
+    const emailRecipients = editor.emailRecipients.filter((recipient) => recipient !== 'individual-learners-emails');


how would we do this from the plugin? since the idea is not to modify the MFE

In that case that won't be an option due to we would have to rewrite BulkEmailForm.js becuase all the logic is here and I don't think so that we have enough time for that

mariajgrimaldi · 2023-12-18T16:40:00Z

src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx

+  /*
+   This will be checking if there are emails added to the emailLearnersList state
+   if so, we will delete emailRecipients "learners" because that is for all learners
+   if not, we will delete the individual-learners-emails from emailRecipients because of we won't use the emails
+  */
+  useEffect(() => {
+    if (emailLearnersList.length && !editor.emailRecipients.includes('individual-learners-emails')) {
+      dispatch(addRecipient('individual-learners-emails'));
+      if (editor.emailRecipients.includes('learners')) {
+        dispatch(removeRecipient('learners'));
+      }
+    } else if (!emailLearnersList.length && editor.emailRecipients.includes('individual-learners-emails')) {
+      dispatch(removeRecipient('individual-learners-emails'));
+    }
+  }, [dispatch, editor.emailRecipients, emailLearnersList]);
+
+  // When the user selects an email from input autocomplete list
+  const handleEmailLearnersSelected = (emailSelected) => {
+    const [firstItem] = emailSelected;
+    if (firstItem) {
+      setEmailLearnersList([...emailLearnersList, firstItem]);
+    }
+  };
+
+  // To delete an email from learners list, that list is on the bottom of the input autocomplete
+  const handleDeleteEmailLearnerSelected = (idToDelete) => {
+    const setEmailLearnersListUpdated = emailLearnersList.filter(({ id }) => id !== idToDelete);
+    setEmailLearnersList(setEmailLearnersListUpdated);
+  };


Same question here about not modifying the MFE since that means more maintenance efforts

johnvente · 2023-12-18T17:23:14Z

My main question is about the changes in the MFE not related to the component slots, do we need them here (right in the MFE) so the plugin works? or can they be moved to the plugin?

In this particular case, we need to write changes in the MFE due to the form has a React context that is and Redux shared throw it's component so I need to change in that part. If we don't want to rewrite the mfe anymore we should wrap the all form with the pluggable component and make the form again and that doesn't make sense. So in this particular case we should modify that part of the code

BryanttV · 2023-12-18T20:48:43Z

Hi @johnvente, I tested the functionality and it works correctly. I would like to comment on two things:

To search a user you need to include 3 characters minimum. Is that how it should work?
I think the recipients container should only be displayed once the first user is added 🤔

johnvente · 2023-12-18T21:22:48Z

Hi @johnvente, I tested the functionality and it works correctly. I would like to comment on two things:

To search a user you need to include 3 characters minimum. Is that how it should work?

I think the recipients container should only be displayed once the first user is added 🤔

Hi @BryanttV

Yep, is recommended to have 3 characters minimum should work like that
I can hide the container while it doesn't have emails added

johnvente · 2023-12-29T16:28:23Z

I will close this pull request and create another one with 100% plugins changes
cc: @BryanttV

johnvente added 4 commits December 18, 2023 08:08

feat: send individuals emails

d138ecc

fix: conflicts

3406770

fix: remove console log

2d4ffbf

test: correct test for scheduling data

4f006d9

johnvente requested a review from a team December 18, 2023 14:19

mariajgrimaldi requested changes Dec 18, 2023

View reviewed changes

chore: update package.json

ced6b97

johnvente force-pushed the jv/pluggable-component-slot branch from 70948cd to ee2dffa Compare December 22, 2023 21:34

johnvente closed this Dec 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: send individuals emails with pluggable component #6

feat: send individuals emails with pluggable component #6

johnvente commented Dec 18, 2023 •

edited

Loading

codecov-commenter commented Dec 18, 2023 •

edited

Loading

mariajgrimaldi left a comment

mariajgrimaldi Dec 18, 2023

mariajgrimaldi Dec 18, 2023

mariajgrimaldi Dec 18, 2023

johnvente Dec 18, 2023

mariajgrimaldi Dec 18, 2023

johnvente Dec 18, 2023

mariajgrimaldi Dec 18, 2023

johnvente commented Dec 18, 2023 •

edited

Loading

BryanttV commented Dec 18, 2023

johnvente commented Dec 18, 2023

johnvente commented Dec 29, 2023

		// eslint-disable-next-line no-console
		console.error('error autocomplete input', error.message);

		renderMenuItemChildren={({ name, email }) => (
		<span data-testid="autocomplete-email-option">{name ? `${name} -` : name} {email}</span>

feat: send individuals emails with pluggable component #6

feat: send individuals emails with pluggable component #6

Conversation

johnvente commented Dec 18, 2023 • edited Loading

Description

DEMO

Pluggin loaded

codecov-commenter commented Dec 18, 2023 • edited Loading

Codecov Report

mariajgrimaldi left a comment

Choose a reason for hiding this comment

mariajgrimaldi Dec 18, 2023

Choose a reason for hiding this comment

mariajgrimaldi Dec 18, 2023

Choose a reason for hiding this comment

mariajgrimaldi Dec 18, 2023

Choose a reason for hiding this comment

johnvente Dec 18, 2023

Choose a reason for hiding this comment

mariajgrimaldi Dec 18, 2023

Choose a reason for hiding this comment

johnvente Dec 18, 2023

Choose a reason for hiding this comment

mariajgrimaldi Dec 18, 2023

Choose a reason for hiding this comment

johnvente commented Dec 18, 2023 • edited Loading

BryanttV commented Dec 18, 2023

johnvente commented Dec 18, 2023

johnvente commented Dec 29, 2023

johnvente commented Dec 18, 2023 •

edited

Loading

codecov-commenter commented Dec 18, 2023 •

edited

Loading

johnvente commented Dec 18, 2023 •

edited

Loading