TNO-2831: Vacation Switch feature #2203

areyeslo · 2024-08-22T17:12:10Z

Vacation Switch Mode provides subscribers with the ability to temporarily disable notifications, such as those related to approved/requested transcriptions and evening reports, while they are on vacation.

Subscribers can easily manage their notification preferences through the Settings under My Account section. This feature gives users control to turn off completely notifications.

When Vacation Mode is activated, a message will appear at the top of the banner, informing the user that notifications are turned off. This banner can be closed at any time by the user.

Fosol · 2024-08-22T18:45:42Z

api/net/Areas/Admin/Controllers/ReportController.cs

@@ -332,12 +332,16 @@ public IActionResult GetDashboardReport(int id)
    {
        var report = _reportService.GetDashboardReport(id);

-        var subscribers = report.SubscribersManyToMany.Where(s => s.User!.AccountType != Entities.UserAccountType.Distribution).Select(s => new UserReportModel(s)).ToList();
+        var subscribers = report.SubscribersManyToMany.Where(s => s.User!.AccountType != Entities.UserAccountType.Distribution && s.User!.IsVacationMode != true).Select(s => new UserReportModel(s)).ToList();


Why are filtering on vacation mode here? This endpoint returns all users who are subscribed so that we can display the status.

Fosol · 2024-08-22T18:45:56Z

api/net/Areas/Admin/Controllers/ReportController.cs

        var distributions = report.SubscribersManyToMany.Where(s => s.User!.AccountType == Entities.UserAccountType.Distribution).ToArray();
        foreach (var distribution in distributions)
        {
            // Flatten the list of subscribers by extracting users in a distribution list.
-            subscribers.AddRange(distribution.User!.Distribution.Select(d => new UserReportModel(distribution, d.LinkedUser!)));
+            subscribers.AddRange(


Why are filtering on vacation mode here? This endpoint returns all users who are subscribed so that we can display the status.

Fosol · 2024-08-22T18:46:22Z

api/net/Areas/Subscriber/Controllers/UserController.cs

@@ -91,8 +91,8 @@ public IActionResult Update(UserModel model)
        var user = _userService.FindByUsername(username) ?? throw new NotAuthorizedException($"User [{username}] does not exist");
        var isAdmin = user.Roles.Split(',').Contains($"[{ClientRole.Administrator.GetName()}]");
        if (user.Id != model.Id && !isAdmin) throw new NotAuthorizedException("You are not authorized to update this user.");
-        var result = _userService.UpdatePreferences((User)model) ?? throw new NoContentException("Updated did not return the user");
-        return new JsonResult(new UserModel(result));
+        var updateUser = _userService.UpdateUser((User)model) ?? throw new NoContentException("Updated did not return the user");


We cannot do this it is a security hole.

Fosol · 2024-08-22T18:47:03Z

api/net/Areas/Subscriber/Controllers/WorkOrderController.cs

@@ -140,7 +140,7 @@ public async Task<IActionResult> RequestTranscriptionAsync(long contentId)

            // Send email to requestor to confirm we have receive their request for a transcript.
            var settingValue = _settingService.FindByName(_apiOptions.TranscriptRequestConfirmationKey)?.Value ?? "";
-            if (workOrder.RequestorId.HasValue && int.TryParse(settingValue, out int notificationId))
+            if (workOrder.RequestorId.HasValue && workOrder.Requestor != null && workOrder.Requestor.IsVacationMode != true && int.TryParse(settingValue, out int notificationId))


A user can still request transcripts when on vacation. The vacation mode is only about receiving emails.

Fosol · 2024-08-22T18:48:38Z

app/subscriber/src/features/access-request/constants/defaultUser.ts

@@ -10,6 +10,7 @@ export const defaultUser: IUserModel = {
  firstName: '',
  lastName: '',
  isEnabled: true,
+  isVacationMode: false,


Move vacation mode to user preferences.

Fosol · 2024-08-22T18:49:14Z

libs/net/entities/User.cs

+    /// get/set - Whether the user turns off notifications.
+    /// </summary>
+    [Column("is_vacation_mode")]
+    public bool IsVacationMode {get; set; }


Do not add this property to the root object. Add it to the user preferences.

Added to preferences.

Fosol · 2024-08-22T18:49:33Z

libs/net/dal/Migrations/TNOContextModelSnapshot.cs

@@ -5108,6 +5108,10 @@ protected override void BuildModel(ModelBuilder modelBuilder)
                        .HasColumnType("boolean")
                        .HasColumnName("is_system_account");

+                    b.Property<bool>("IsVacationMode")


We don't need a DB migration for this.

Thank you! Working on the changes

Migration removed.

Fosol · 2024-08-26T14:26:08Z

api/net/Areas/Subscriber/Controllers/WorkOrderController.cs

@@ -147,7 +147,9 @@ public async Task<IActionResult> RequestTranscriptionAsync(long contentId)
                    NotificationId = notificationId,
                    ContentId = contentId,
                    RequestorId = workOrder.RequestorId,
-                    To = !String.IsNullOrWhiteSpace(user.PreferredEmail) ? user.PreferredEmail : user.Email,
+                    To = user != null && !user.IsVacationMode()


Not needed. Vacation mode is just to turn off emails.

I understood this part would send email using the email account of To property. So I checked the email account is not under VacationMode.

Fosol · 2024-08-26T14:31:26Z

app/subscriber/src/components/user-profile/UserProfile.tsx

+    }
+  }, [isVacationMode]);
+
+  const handleVacationModeToggle = () => {


Use React.useCallback or this function will generate a render each time

Fosol · 2024-08-26T14:36:17Z

app/subscriber/src/features/settings/MyAccountSettings.tsx

+      // Handle the error, if needed
+      console.error('Failed to update user:', error);
+    }
+  }, [profile, impersonate, isVacationMode, storeImpersonate, storeMyProfile, updateUser]);


Move profile, impersonate and isVacationMode to parameters in the function will reduce the rerendering required to keep this function in sync with the variables. You can just call the function like this toggleVacationMode(impersonate ?? profile, !isVacationMode)

Fosol · 2024-08-26T14:37:31Z

app/subscriber/src/features/settings/MyAccountSettings.tsx

+    const user = createUser();
+
+    try {
+      !!impersonate ? storeImpersonate(user) : storeMyProfile(user);


This logic is already performed by the useUser hook. I don't see a need to do it again.

Fosol · 2024-08-26T14:44:51Z

services/net/notification/NotificationManager.cs

            }
            else
            {
-                return new[] { subscriber.User };
+                return !subscriber.User.IsVacationMode()


You have already filtered out users that are on vacation mode on line 397. Why are you doing it again?

if userAccountType is type of Distribution then we will discard those emails with IsVacationMode. Otherwise, if userAccountType is not a single user, then added if does not have IsVacationMode. We want to discard email accounts with IsVacationMode in both conditions. Is this understanding not right?

Fosol · 2024-08-26T14:53:46Z

services/net/reporting/ReportingManager.cs

@@ -1016,17 +1017,21 @@ private async Task<Dictionary<EmailSentTo, List<UserEmail>>> GetEmailAddressesAs
        }
        else
        {
-            switch (sendTo)
+            var user = await this.Api.GetUserAsync(userId);


This will add a lot of additional requests for each report. We already do a join on the user table to get all the user's email addresses. You can update the model to include the vacation mode.

Fosol · 2024-08-26T19:32:09Z

services/net/reporting/ReportingManager.cs

@@ -974,7 +974,7 @@ private async Task<Dictionary<EmailSentTo, List<UserEmail>>> GetEmailAddressesAs
            if (user.User == null) throw new InvalidOperationException("Report subscriber is missing user information");

            // Only include users who have not received an email yet.
-            var (a, b, c) = await GetEmailAddressesAsync(user.UserId, user.User.GetEmail(), user.User.AccountType, user.SendTo);
+            var (a, b, c) = await GetEmailAddressesAsync(user.UserId, user.User.GetEmail(), user.User.IsVacationMode(), user.User.AccountType, user.SendTo);


This is not needed. You already filtered out users on vacation before calling this method.

jdtoombs · 2024-08-26T20:52:22Z

@areyeslo - published tno-core@0.1.136 just need to update your package.json's!

Fosol · 2024-08-26T21:45:49Z

app/editor/package.json

@@ -61,7 +61,7 @@
    "redux-logger": "3.0.6",
    "styled-components": "6.1.11",
    "stylis": "4.3.2",
-    "tno-core": "0.1.135"
+    "tno-core": "./tno-core-0.1.136.tgz"


Need to correct

Fosol · 2024-08-26T21:45:55Z

app/editor/tno-core-0.1.135.tgz

Fosol · 2024-08-26T21:45:57Z

app/editor/tno-core-0.1.136.tgz

Fosol · 2024-08-26T21:46:09Z

app/subscriber/tno-core-0.1.134.tgz

Fosol · 2024-08-26T21:46:14Z

app/subscriber/tno-core-0.1.135.tgz

Fosol · 2024-08-26T21:46:16Z

app/subscriber/tno-core-0.1.136.tgz

Fosol · 2024-08-26T22:34:37Z

app/editor/tno-core-0.1.136.tgz

Fosol · 2024-08-26T22:34:45Z

app/editor/yarn.lock

@@ -11074,7 +11074,7 @@ __metadata:
    sass-extract-loader: 1.1.0
    styled-components: 6.1.11
    stylis: 4.3.2
-    tno-core: ./tno-core-0.1.134.tgz
+    tno-core: ./tno-core-0.1.136.tgz


Fosol · 2024-08-26T22:34:55Z

app/subscriber/tno-core-0.1.136.tgz

Fosol · 2024-08-26T22:35:03Z

app/subscriber/yarn.lock

@@ -10698,7 +10697,7 @@ __metadata:
    sheetjs: "file:packages/xlsx-0.20.1.tgz"
    styled-components: 6.1.11
    stylis: 4.3.2
-    tno-core: ./tno-core-0.1.134.tgz
+    tno-core: ./tno-core-0.1.136.tgz


Fosol · 2024-08-26T22:48:06Z

app/subscriber/yarn.lock

-"tno-core@file:./tno-core-0.1.134.tgz::locator=mmi-subscriber-app%40workspace%3A.":
-  version: 0.1.134
-  resolution: "tno-core@file:./tno-core-0.1.134.tgz::locator=mmi-subscriber-app%40workspace%3A."
+"tno-core@file:./tno-core-0.1.136.tgz::locator=mmi-subscriber-app%40workspace%3A.":


You didn't correctly run yarn install to fix this lock. You manually changed it.

Fosol · 2024-08-26T22:48:16Z

app/editor/yarn.lock

-  resolution: "tno-core@file:./tno-core-0.1.134.tgz::locator=mmi-editor-app%40workspace%3A."
+"tno-core@file:./tno-core-0.1.136.tgz::locator=mmi-editor-app%40workspace%3A.":
+  version: 0.1.136
+  resolution: "tno-core@file:./tno-core-0.1.136.tgz::locator=mmi-editor-app%40workspace%3A."


You didn't correctly run yarn install to fix this lock. You manually changed it.

The last push is executing yarn install.

areyeslo force-pushed the TNO-2831-VacationSwitch branch from 94b5c6b to 90a96d7 Compare August 22, 2024 17:22

areyeslo marked this pull request as ready for review August 22, 2024 17:57

areyeslo requested a review from Fosol August 22, 2024 17:58

Fosol assigned areyeslo Aug 22, 2024

Fosol requested changes Aug 22, 2024

View reviewed changes

areyeslo marked this pull request as draft August 22, 2024 20:09

areyeslo force-pushed the TNO-2831-VacationSwitch branch 3 times, most recently from 37e9a72 to cad54ff Compare August 23, 2024 22:02

areyeslo removed the DB Migration A DB Migration may require refreshing or simply updating your database. label Aug 23, 2024

areyeslo force-pushed the TNO-2831-VacationSwitch branch 7 times, most recently from 4f7b240 to 41cd959 Compare August 23, 2024 23:42

areyeslo marked this pull request as ready for review August 23, 2024 23:51

areyeslo requested a review from Fosol August 23, 2024 23:52

Fosol requested changes Aug 26, 2024

View reviewed changes

Vacation Mode feature to turn off notifications.

edae7f0

areyeslo force-pushed the TNO-2831-VacationSwitch branch 2 times, most recently from 5399da8 to e78b39c Compare August 26, 2024 17:09

Changes due to code review

71182ab

areyeslo force-pushed the TNO-2831-VacationSwitch branch from e78b39c to 71182ab Compare August 26, 2024 18:36

Fosol requested changes Aug 26, 2024

View reviewed changes

Adding bottom padding to show the toggle button in mobile devices.

3dda3ca

areyeslo force-pushed the TNO-2831-VacationSwitch branch from 01299e7 to 3dda3ca Compare August 26, 2024 19:35

removing extra filtering in reportingManager

a0ba8d6

areyeslo requested a review from Fosol August 26, 2024 20:00

areyeslo force-pushed the TNO-2831-VacationSwitch branch from f5c8c3e to 15de8f7 Compare August 26, 2024 21:17

Upgrade tno-core version to 0.1.136

7b2c454

areyeslo force-pushed the TNO-2831-VacationSwitch branch from 15de8f7 to 7b2c454 Compare August 26, 2024 21:23

Fosol requested changes Aug 26, 2024

View reviewed changes

areyeslo added 3 commits August 26, 2024 15:54

Remove tgz files.

44324e5

make pack with 0.1.136 version.

37c2e9e

update package.json

a8a6b06

areyeslo requested a review from Fosol August 26, 2024 22:33

Fosol requested changes Aug 26, 2024

View reviewed changes

Modiying yarn file for subscriber and editor.

d14408d

areyeslo force-pushed the TNO-2831-VacationSwitch branch from 8f488bb to d14408d Compare August 26, 2024 22:51

Fosol approved these changes Aug 26, 2024

View reviewed changes

Fosol merged commit f205dd4 into dev Aug 26, 2024
7 checks passed

Fosol deleted the TNO-2831-VacationSwitch branch August 26, 2024 23:01

TNO-2831: Vacation Switch feature #2203

TNO-2831: Vacation Switch feature #2203

Conversation

areyeslo commented Aug 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdtoombs commented Aug 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment