🎁 adds batch email, depositor email, and user stat to Account Settings #2429

ShanaLMoore · 2025-01-24T19:22:15Z

Issue:

🎁 Make batch email, depositor email and user stats account settings notch8/palni_palci_knapsack#209

Feature flags are now visible in ADMIN DASHBOARD > SETTINGS > ACCOUNT. When enabled, they will trigger the relevant background jobs.

Additionally, if disabled a user will not see batch email frequency in their profiles. It also removed their ability to set it:

PROFILE SHOW PG

WHEN DISABLED

WHEN ENABLED

PROFILE EDIT PG

WHEN DISABLED

WHEN ENABLED

NOTES

🎁 add dynamic toggling for job scheduling using environment variables

abf36be

Updated find_or_schedule_jobs to dynamically include jobs based on environment variables.
Introduced the following environment flags to control job scheduling:
- ENABLE_BATCH_EMAIL for BatchEmailNotificationJob
- ENABLE_DEPOSITOR_EMAIL for DepositorEmailNotificationJob
- ENABLE_USER_STAT for UserStatCollectionJob
This allows buggy jobs to be disabled temporarily without modifying the codebase.
Default behavior ensures only non-buggy jobs are scheduled when flags are not set.

♻️ Refactor to remove ENV vars in favor of account settings pattern

52e90c5

Adds batch_email_notifications, depositor_email_notifications, and user_analystics to account settings
only displays batch email frequency if batch email notifications are enabled from account settings

🧹 Automatically schedule jobs when account settings change

06afd3e

When certain account settings (user_analytics, batch_email_notifications,
or depositor_email_notifications) are updated, the system will now
automatically schedule the corresponding background jobs. This eliminates
the need for a server restart to activate these features.

Added after_save callback to detect settings changes
Added logic to compare relevant settings before and after save
Triggers find_or_schedule_jobs when settings change

This ensures features like user analytics start working immediately
after being enabled through the settings interface.

♻️ Refactor account settings change detection for better readability

50040bc

Simplifies the #schedule_jobs_if_settings_changed method to be more
intuitive and easier to maintain:

Replace database column check with direct settings check
Add descriptive variable names and comments
Break complex logic into clear, distinct steps
Improve code organization with early returns

The functionality remains the same, but the code is now more
self-documenting and easier to understand.

✅ update schedule_recurring_jobs specs

30e144c

Add test contexts for enabled and disabled account settings
Verify that EmbargoAutoExpiryJob and LeaseAutoExpiryJob always run
Verify that BatchEmailNotificationJob, DepositorEmailNotificationJob,
and UserStatCollectionJob only run when their respective settings
are enabled

✅ update account settings specs

20b1c97

✅ Update create_account_spec.rb

b2794c5

✅ Mock find_or_schedule_jobs in account endpoints spec

fe02a77

The account endpoints spec was failing because it attempted to
schedule jobs before the tenant schema was created. By mocking
find_or_schedule_jobs in this specific test, we avoid the schema
error while preserving the actual job scheduling behavior in other
tests.

github-actions · 2025-01-24T20:05:59Z

Test Results

3 files ±0 3 suites ±0 17m 24s ⏱️ -19s
2 057 tests +1 2 001 ✅ +1 56 💤 ±0 0 ❌ ±0
2 084 runs +1 2 026 ✅ +1 58 💤 ±0 0 ❌ ±0

Results for commit fe02a77. ± Comparison against base commit 9f98d14.

This pull request removes 43 and adds 44 tests. Note that renamed tests count towards both.

spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 8d9f7542-ff73-4fd7-b1c5-5f20baea919c
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit 03ea5077-2ab1-46fd-be8b-6a2ca0d2e0db
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read ca308801-fef3-416e-b20a-c84652277af4
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update c9b4eed5-2fd5-4aea-aae2-dfcbe6ea2b30
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 706b1166-6408-45b1-8a61-f67d0bc48f5f
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit db379bca-edcc-47df-84bc-7771a604e36c
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read 5b5d1ba3-0f64-4997-8d02-ca135aa482d3
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 4e756118-09cf-43b0-abf2-3ec52566c799
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy 5fc60b0f-fca5-497d-9088-ccf8537d436a
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit 77d366d9-1b37-4bee-8e14-e5eaa2b32b95
…

spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 91577bb0-78ce-4323-898a-402d3a993b29
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit f20bd307-96af-4543-b46d-14e28f7e7347
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read 85b3ec5e-524f-42ef-9d26-c96c0d1df1b5
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update fd1b376d-3ab1-40ef-a3bf-98d2d400ae9e
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 6931130d-7437-4803-b259-0e7572b5620c
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit 35e4b88c-3737-417a-91ba-b3d8bff757cd
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read d1ea4aab-9797-4fdc-be24-940f0a5a0acc
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 014e6d32-ffc9-4c0d-a160-443c8fa0ed99
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy d1453cf5-fa41-4f10-a259-0bdd23d9f5a3
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit 0cc47d99-5530-443e-b277-18e72afb78c5
…

♻️ This comment has been updated with latest results.

orangewolf

The concept here is fine, but the implementation needs rework. “ENABLED” is implied. The flag is either on or off and that word is just extra. Also, all settings in Hyku should use AccountSettings. The automatically handles the Boolean issue, makes them settable per tenant or globally and provides consistency.

orangewolf · 2025-01-24T20:22:12Z

I recommend BATCH_EMAIL_NOTIFICATION for the env var name

ShanaLMoore · 2025-01-24T21:45:05Z

The concept here is fine, but the implementation needs rework. “ENABLED” is implied. The flag is either on or off and that word is just extra. Also, all settings in Hyku should use AccountSettings. The automatically handles the Boolean issue, makes them settable per tenant or globally and provides consistency.

Thanks for the reminder. That's definitely easier to manage. I'll make the updates as suggested.

ShanaLMoore · 2025-01-25T00:16:29Z

spec/features/accounts_spec.rb

@@ -16,6 +16,7 @@
      allow(Apartment::Tenant).to receive(:switch).with(account.tenant) do |&block|
        block.call
      end
+      allow_any_instance_of(Account).to receive(:find_or_schedule_jobs)


Thoughts about this? This spec was failing because of the after_save addition. cc @orangewolf

Yes, this seems fine

- Updated `find_or_schedule_jobs` to dynamically include jobs based on environment variables. - Introduced the following environment flags to control job scheduling: - `ENABLE_BATCH_EMAIL` for `BatchEmailNotificationJob` - `ENABLE_DEPOSITOR_EMAIL` for `DepositorEmailNotificationJob` - `ENABLE_USER_STAT` for `UserStatCollectionJob` - This allows buggy jobs to be disabled temporarily without modifying the codebase. - Default behavior ensures only non-buggy jobs are scheduled when flags are not set.

- Adds batch_email_notifications, depositor_email_notifications, and user_analystics to account settings - only displays batch email frequency if batch email notifications are enabled from account settings

When certain account settings (user_analytics, batch_email_notifications, or depositor_email_notifications) are updated, the system will now automatically schedule the corresponding background jobs. This eliminates the need for a server restart to activate these features. - Added after_save callback to detect settings changes - Added logic to compare relevant settings before and after save - Triggers find_or_schedule_jobs when settings change This ensures features like user analytics start working immediately after being enabled through the settings interface.

Simplifies the #schedule_jobs_if_settings_changed method to be more intuitive and easier to maintain: - Replace database column check with direct settings check - Add descriptive variable names and comments - Break complex logic into clear, distinct steps - Improve code organization with early returns The functionality remains the same, but the code is now more self-documenting and easier to understand.

- Add test contexts for enabled and disabled account settings - Verify that EmbargoAutoExpiryJob and LeaseAutoExpiryJob always run - Verify that BatchEmailNotificationJob, DepositorEmailNotificationJob, and UserStatCollectionJob only run when their respective settings are enabled

The account endpoints spec was failing because it attempted to schedule jobs before the tenant schema was created. By mocking find_or_schedule_jobs in this specific test, we avoid the schema error while preserving the actual job scheduling behavior in other tests.

orangewolf

This is great Shana!

orangewolf · 2025-01-27T17:08:58Z

spec/features/accounts_spec.rb

@@ -16,6 +16,7 @@
      allow(Apartment::Tenant).to receive(:switch).with(account.tenant) do |&block|
        block.call
      end
+      allow_any_instance_of(Account).to receive(:find_or_schedule_jobs)


Yes, this seems fine

ShanaLMoore added ignore-for-release ignore this for release notes minor-ver for release notes and removed ignore-for-release ignore this for release notes labels Jan 24, 2025

ShanaLMoore requested a review from orangewolf January 24, 2025 19:23

orangewolf requested changes Jan 24, 2025

View reviewed changes

ShanaLMoore marked this pull request as draft January 24, 2025 22:45

ShanaLMoore marked this pull request as ready for review January 24, 2025 23:03

ShanaLMoore changed the title ~~🧹 add dynamic toggling for job scheduling using environment vars~~ 🎁 adds dynamic toggling for job scheduling using environment vars Jan 24, 2025

ShanaLMoore changed the title ~~🎁 adds dynamic toggling for job scheduling using environment vars~~ 🎁 adds dynamic toggling for job scheduling Jan 24, 2025

ShanaLMoore changed the title ~~🎁 adds dynamic toggling for job scheduling~~ 🎁 adds batch email, depositor email, and user stat Account Settings Jan 24, 2025

ShanaLMoore requested a review from orangewolf January 24, 2025 23:05

ShanaLMoore changed the title ~~🎁 adds batch email, depositor email, and user stat Account Settings~~ 🎁 adds batch email, depositor email, and user stat to Account Settings Jan 24, 2025

ShanaLMoore marked this pull request as draft January 24, 2025 23:36

ShanaLMoore commented Jan 25, 2025

View reviewed changes

Shana Moore added 8 commits January 24, 2025 16:18

♻️ Refactor to remove ENV vars in favor of account settings pattern

52e90c5

- Adds batch_email_notifications, depositor_email_notifications, and user_analystics to account settings - only displays batch email frequency if batch email notifications are enabled from account settings

✅ update account settings specs

20b1c97

✅ Update create_account_spec.rb

b2794c5

ShanaLMoore force-pushed the disable-pitt-jobs branch from e4695b4 to fe02a77 Compare January 25, 2025 00:21

ShanaLMoore marked this pull request as ready for review January 27, 2025 15:37

orangewolf approved these changes Jan 27, 2025

View reviewed changes

ShanaLMoore merged commit ca7acdb into main Jan 27, 2025
8 checks passed

ShanaLMoore deleted the disable-pitt-jobs branch January 27, 2025 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎁 adds batch email, depositor email, and user stat to Account Settings #2429

🎁 adds batch email, depositor email, and user stat to Account Settings #2429

ShanaLMoore commented Jan 24, 2025 •

edited

Loading

github-actions bot commented Jan 24, 2025 •

edited

Loading

orangewolf left a comment

orangewolf commented Jan 24, 2025

ShanaLMoore commented Jan 24, 2025

ShanaLMoore Jan 25, 2025

orangewolf Jan 27, 2025

orangewolf left a comment

orangewolf Jan 27, 2025

🎁 adds batch email, depositor email, and user stat to Account Settings #2429

🎁 adds batch email, depositor email, and user stat to Account Settings #2429

Conversation

ShanaLMoore commented Jan 24, 2025 • edited Loading

PROFILE SHOW PG

WHEN DISABLED

WHEN ENABLED

PROFILE EDIT PG

WHEN DISABLED

WHEN ENABLED

NOTES

🎁 add dynamic toggling for job scheduling using environment variables

♻️ Refactor to remove ENV vars in favor of account settings pattern

🧹 Automatically schedule jobs when account settings change

♻️ Refactor account settings change detection for better readability

✅ update schedule_recurring_jobs specs

✅ update account settings specs

✅ Update create_account_spec.rb

✅ Mock find_or_schedule_jobs in account endpoints spec

github-actions bot commented Jan 24, 2025 • edited Loading

Test Results

orangewolf left a comment

Choose a reason for hiding this comment

orangewolf commented Jan 24, 2025

ShanaLMoore commented Jan 24, 2025

ShanaLMoore Jan 25, 2025

Choose a reason for hiding this comment

orangewolf Jan 27, 2025

Choose a reason for hiding this comment

orangewolf left a comment

Choose a reason for hiding this comment

orangewolf Jan 27, 2025

Choose a reason for hiding this comment

ShanaLMoore commented Jan 24, 2025 •

edited

Loading

github-actions bot commented Jan 24, 2025 •

edited

Loading