feat: prefer after_create instead of after-commit due to transactional guarentees

[AlexRiedler]

Generally after_commit can be dangerous as it means its completely non-transactional.

With the switch to using postgres backed job queue library (instead of redis); it now makes sense to queue the job within the commit in a after_create. Generally all after_commit has no guarantee of running and only things like logging really make sense in them (or anything that can be removed without consequence).

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

[cassiascheffer]

Wording comments. Lint rules should be written in imperative language to make it clear that they are (almost) never optional.

[AlexRiedler]

wording changes.

[hannahyeates]

@AlexRiedler it would be worth calling out that there is not a built-in before_commit callback: https://guides.rubyonrails.org/active_record_callbacks.html#available-callbacks. Can we recommend that people use after_save instead?

[AlexRiedler]

@hannahyeates it was removed? https://apidock.com/rails/v6.1.3.1/ActiveRecord/Transactions/ClassMethods/before_commit 😢

[AlexRiedler]

There is even a reference to it still in the current rails docs: https://api.rubyonrails.org/classes/ActiveRecord/Callbacks.html

[hannahyeates]

@AlexRiedler I think after_commit is a private method. It doesn't show up in the list of exposed callbacks:
. My suspicion that before_commit is private comes from this GH issue: rails/rails#44458

I think the before_commit hook you referenced above is on the ActiveRecord::Transaction class: https://apidock.com/rails/v5.2.3/ActiveRecord/Transactions/ClassMethods/before_commit. The after_commit hook that would be disallowed here is on the ActiveRecord::Callbacks class: https://apidock.com/rails/v5.2.3/ActiveRecord/Callbacks

I saw this PR and tried to swap after_commit for before_commit and it didn't work because they aren't a 1:1 swap from the looks of it. Instead of recommending that we use the transaction level hook instead of the object level hook, can we recommend a safer model hook so that it is an easier fix to make? I'm not sure of the lift on doing this refactor, so if we really want to leverage before commit hooks, maybe we include some documentation for how to make this change?

[hannahyeates]

I think I'm wrong about the private method thing because the latest docs have it public: https://apidock.com/rails/v5.2.3/ActiveRecord/Transactions/ClassMethods/before_commit

[AlexRiedler]

oh okay, lets just change it to after_create then (updating)

[desheikh]

A note here, devs cannot just switch after_commit to after_save and expect things to continue working exactly the same.

In the case of triggering jobs after a model update, there is a high chance the worker gets instantiated before the transaction completes and uses a stale version of the model. This is the reason folks landed on after_commit in the first place.

I'm afraid the cops doesn't make that clear and there's a good chance it'll result in more breaking behaviour.

[AlexRiedler]

@desheikh good point; maybe there is a better way to word this hmmm...; or a way to link to something with good details around this cause it is nuance.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.