Add one_email_per_occurrence option and migration task

[imageaid]

Motivation:

We are happily using Solid Errors in our production app but because that app connects to some external APIs we can run into situations where we are sent a lot of emails for the same error's reoccurrence.

Details:

Introduced a new configuration option one_email_per_occurrence that serves to limit email notifications to one per occurrence. If an error is resolved but reoccurs, an email will be sent, again, for that first reoccurrence.

In order to do this, a migration was created to add a new column/attribute to the solid_errors table called prev_resolved_at.

This column/attribute is updated to match the resolved_at value whenever it is set - but it is never nill'ed out like resolved_at is when the issue is resolved.

A Rake task, solid_errors:install_migrations, was also added to copy any new migrations from the lib/solid_errors/db/migrate directory and then immediately run them.

The README was updated as was the error controller to support the new feature.

The .gemspec was also updated to add a note reminding users to ensure they've run the task to copy over the migration(s).

Please let me know if there's anything you'd change, do better, etc. Thank you for your excellent work on Solid Errors. We are grateful to have it!

[imageaid]

I like this! Thanks for such a clear and thorough PR. I've made a few comments for things to tweak to get it over the line.

Thank you so much for taking the time to review this and to add all your helpful edits and comments. Much much appreciated!

I am not sure if you need me to/if I should make some commits in my branch and redo my PR (this is my first PR to a gem so please forgive any stupidity or missteps on my part!).

Thanks, again, so much for your work. Please know how much my team appreciates Solid Errors!

[fractaledmind]

Yeah, you've done such a great job with the PR that I'd like you to finish it yourself and have full credit!

[imageaid]

Yeah, you've done such a great job with the PR that is like you to finish it yourself and have full credit!

AWESOME! Man, really appreciate it. I will make the changes and commit them shortly

[imageaid]

For the prev_resolved_at, I just made a commit that might be a better way? Let me know your thoughts.

Basically, I removed that element from the buttons in the _action and _row view files as well as the controller. Instead, I set it in the Subscriber just before the resolved_at value is nilled out.

Maybe less intrusive and more seamless?

Totally open to other alternatives of course!

Finally, for the migration, I just kept the rake task the same in terms of copying the migration files over. But, I killed the running of them (I have some commented out code as an alternative approach to run just the specific migration if desired) because Rails will, of course, scream and yell at us that we have pending migrations to run ... duh on my part 🤣

[fractaledmind]

We still need a note in the README explaining the behavior for email sending

[imageaid]

We still need a note in the README explaining the behavior for email sending

Just pushed a commit with an edited note about sending emails. Let me know if you think that fits the bill or needs some added massaging, etc.

[fractaledmind]

I know we are taking a while polishing the final exacting details, but we are close and this is a great PR. Thanks for the patience

[imageaid]

I know we are taking a while polishing the final exacting details, but we are close and this is a great PR. Thanks for the patience

Happy to! I like getting things polished and as close to ideal as possible. No problems at all on my end! I'll commit again later tonight. Time to go walk and feed the pups!

[imageaid]

Ok, Stephen, I think I got that last round of changes in place. Thanks for catching the spelling error(s) 🤦🏻

Let me know how that README line sounds about the post-install message, etc. Or any other changes you find need made. Thanks!

[fractaledmind]

I want to chat with @bensheldon some about how he manages versions with schema changes before merging and releasing. I want a clear plan.

[bensheldon]

👋🏻 Happy to chat. I can try to sketch out a few things (I've been wanting to blog about this forever!) and maybe invite further chatting.

Upfront: GoodJob does a lot of migrations. A lot! I want to say there have been about 30 migration files over its lifespan so far. That's more than like Active Storage which has... 4. So take this with a grain of salt if your gem is more like Active Storage than GoodJob.

• Required database migrations are breaking changes, so that means if I make a breaking change it warrants a major version release. I try very hard not to allow a database migration to be required.
• It takes more work to make migrations (or more importantly, their absence) non-breaking and optional. This largely means doing schema checks (does this column exist? if so, then do x, if not, then don't), and writing fairly defensive code (e.g. constructing attribute hashes and array accessors for Active Record models instead of using accessor methods)
• I make sure that the migrations themselves are idempotent
  • The first time developer rails g good_job:install command installs a single migration that should get the developer the most up-to-date schema.
  • The rails g good_job:update which installs all the update migrations (which could be several) for the current major version. This one gets a little weird because some of the migrations must be noops if they did the former first-time good_job::install migration. So I add a schema check to make them noops. The Rails generator command will detect duplicate migrations based on the name, so it is idempotent.
• When I do release a major version, I squash down all of the "update" migrations. I do this because I would not be happy, as a first-time user of GoodJob, to immediately be confronted with 30 migrations. The downside of this is that people will upgrade to a major version without applying all the previous major version's migrations and then ask why GoodJob is broken (this is why I do x.99 releases to try to be explicit about the upgrade path). I get many support issues on this when I cut a major release.
• I have a whole set of generator tests that create a new Rails app and validate that the install and upgrade migrations are in sync. It's very slow but has saved my bacon a lot.

Sorry, that's a wall of text 😅

[imageaid]

I pushed a small update (well, two) yesterday evening. These two ensure that, in the current approach to the migration, it does copy over correctly.

Note: I did bump the version on this branch (0.4.3.003) so I could ensure that my bundle update grabbed the newest code. That can, of course, be reverted any time.

Let me know, please, if you have any thoughts or approaches you'd like for this PR's final push. Thanks!

[fractaledmind]

I'm on vacation for the week. Will wrap up when I get back. Thanks for the great work.