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

[Feature] Send 'new pool' emails overnight #11804

Merged
merged 8 commits into from
Nov 1, 2024
Merged

Conversation

petertgiles
Copy link
Contributor

πŸ€– Resolves #11709

πŸ‘‹ Introduction

Sends "new pool published" emails overnight instead of immediately.

πŸ•΅οΈ Details

One of the AC is:

Increase the time limit on queued jobs from 10 minutes to 30 minutes

This is no longer relevant since the script is running from the cron job and not a task scheduler worker. It no longer has a time limit - I think. πŸ˜…

πŸ§ͺ Testing

  1. Set the GC Notify "team" API key
  2. Rebuild the app
  3. Start the queue worker
  4. Set your real work email address to a user and opt in to JOB_ALERT email alerts
  5. Publish a pool with the IT_JOBS publishing group
  6. Run ./artisan send-notifications:pool-published
  7. Ensure you get a notification email

Also:

  • More than one published pool -> more than one email
  • OTHER publishing group -> no email

@brindasasi brindasasi self-requested a review October 28, 2024 14:23
Copy link
Contributor

@brindasasi brindasasi left a comment

Choose a reason for hiding this comment

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

image
22k notifications got processed in Dev without any issues.
Everything looks good. This is good to go once the linting & failing tests are fixed.
I did verify I don't get email for "Other" publishing group.

$startOfSpan = Carbon::now()->subHours(24); // assuming the last reporting job ran about 24 hours ago
$this->info("Finding pools published between $startOfSpan and $endOfSpan.");

$poolsPublishedRecently = Pool::query()
Copy link
Contributor

Choose a reason for hiding this comment

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

one more question .. this pr checks the pools published within 24 hours but doesn't check for open pools.
In the use case , they have published something accidentally and they closed it immediately, we would still send notification for that closed pool.

I'm wondering should we add that check for only active pools?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. If I had remembered to update the tests first, this would have been caught. 🀦

Copy link
Contributor

@brindasasi brindasasi left a comment

Choose a reason for hiding this comment

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

This is working great! Awesome πŸ₯‡

@petertgiles petertgiles added this pull request to the merge queue Nov 1, 2024
Merged via the queue into main with commit 3cbb385 Nov 1, 2024
6 checks passed
@petertgiles petertgiles deleted the 11709-overnight-emails branch November 1, 2024 14:09
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.

✨ Send new poster notifications overnight instead of immediately
2 participants