fix: remove uploaded files if transaction is rolled back

[fsateler]

This prevents accumulating stale files when transaction is aborted.

Depends on #2545 , restores functionality added by #2209

[mshibuya]

These tests need to be added back to ensure that this change has the effect intended in #2209.
88e3717#diff-48bc92466ff9653e1ae5c5b60134a0c7c631b0d414f0d9b0b4642e84097a650bL790-L828

In addition to that, you need to add tests for the functionality stated in #2544.

[fsateler]

These tests need to be added back to ensure that this change has the effect intended in #2209.
88e3717#diff-48bc92466ff9653e1ae5c5b60134a0c7c631b0d414f0d9b0b4642e84097a650bL790-L828

The tests are there. They are reverted in the first commit but re-added in the second.

In addition to that, you need to add tests for the functionality stated in #2544.

The tests are already there. By reading public_path instead of file_path, the tests were reading temporary files instead of the definitive one (ie, they were reading the bogus urls).

Do you have a specific test in mind?

[mshibuya]

These tests need to be added back...

The tests are there.

Sorry I overlooked, please ignore this.

Do you have a specific test in mind?

These points are stated in #2544, so we should have corresponding test cases to ensure they are resolved by this change.

For example, reading my_object.file_url.url will return a bogus url if I read before commit.
For example, if there is a network failure. This causes the transaction to not be rolled back and invalid records being recorded.

[fsateler]

These points are stated in #2544, so we should have corresponding test cases to ensure they are resolved by this change.

For example, reading my_object.file_url.url will return a bogus url if I read before commit.
For example, if there is a network failure. This causes the transaction to not be rolled back and invalid records being recorded.

Done.

[tomprats]

Would just like to echo that moving this back to a before_save is very helpful. I've been getting a mysterious: OpenURI::HTTPError: 403 Forbidden in a background job triggered in an after_commit ever since I upgraded to 2. What confused me even more was that it always succeeds when retried.

Appreciate the work on this!

[mshibuya]

Thank you for your work 🙌