Commit Graph

531 Commits

Author SHA1 Message Date
Gerhard Schlager edc8d58ac3 FEATURE: Add site setting to disable staged user cleanup
... and disabled the cleanup during imports, otherwise a running Sidekiq might delete users before posts are created
2020-03-09 10:26:41 +01:00
Sam Saffron 372f6f4f22
FEATURE: limit number of notifications per user to 10,000
Introduces a new site setting `max_notifications_per_user`.

Out-of-the-box this is set to 10,000. If a user exceeds this number of
notifications, we will delete the oldest notifications keeping only 10,000.

To disable this safeguard set the setting to 0.

Enforcement happens weekly.

This is in place to protect the system from pathological states where a
single user has enormous amounts of notifications causing various queries
to time out. In practice nobody looks back more than a few hundred notifications.
2020-02-24 11:42:50 +11:00
Roman Rizzi 2ee6a615b7
FEATURE: Send suspect users to the review queue (#8811) 2020-01-29 15:38:27 -03:00
Gerhard Schlager 9621af2214 FIX: Award "User of the month" badge at the beginning of month
Previously the badge was granted one month after the last time the badge was granted. The exact date shifted by one day each month. The new logic tries to grant the badge always at the beginning of a new month by looking at new users of the previous month. The "granted at" date is set to the end of the previous month.
2020-01-28 22:59:13 +01:00
David Taylor a9d0d55817 FEATURE: Add message to log when admins are automatically deactivated 2020-01-28 12:16:24 +00:00
Gerhard Schlager ea11ad4d99 DEV: Drop unused columns 2020-01-27 15:28:56 +01:00
Gerhard Schlager f216c6d60b FEATURE: Drop "backup" schema 7 days after restore
The "backup" schema is used to rollback a failed restore. It isn't useful after a longer period of time and turns into a waste of disk space.
2020-01-16 17:48:47 +01:00
Martin Brennan 7c32411881
FEATURE: Secure media allowing duplicated uploads with category-level privacy and post-based access rules (#8664)
### General Changes and Duplication

* We now consider a post `with_secure_media?` if it is in a read-restricted category.
* When uploading we now set an upload's secure status straight away.
* When uploading if `SiteSetting.secure_media` is enabled, we do not check to see if the upload already exists using the `sha1` digest of the upload. The `sha1` column of the upload is filled with a `SecureRandom.hex(20)` value which is the same length as `Upload::SHA1_LENGTH`. The `original_sha1` column is filled with the _real_ sha1 digest of the file. 
* Whether an upload `should_be_secure?` is now determined by whether the `access_control_post` is `with_secure_media?` (if there is no access control post then we leave the secure status as is).
* When serializing the upload, we now cook the URL if the upload is secure. This is so it shows up correctly in the composer preview, because we set secure status on upload.

### Viewing Secure Media

* The secure-media-upload URL will take the post that the upload is attached to into account via `Guardian.can_see?` for access permissions
* If there is no `access_control_post` then we just deliver the media. This should be a rare occurrance and shouldn't cause issues as the `access_control_post` is set when `link_post_uploads` is called via `CookedPostProcessor`

### Removed

We no longer do any of these because we do not reuse uploads by sha1 if secure media is enabled.

* We no longer have a way to prevent cross-posting of a secure upload from a private context to a public context.
* We no longer have to set `secure: false` for uploads when uploading for a theme component.
2020-01-16 13:50:27 +10:00
David Taylor cff6e941de
PERF: Cache ranks for featured badges, to simplify user serialization (#8698) 2020-01-14 14:26:49 +00:00
Joffrey JAFFEUX 0d3d2c43a0
DEV: s/\$redis/Discourse\.redis (#8431)
This commit also adds a rubocop rule to prevent global variables.
2019-12-03 10:05:53 +01:00
David Taylor e1fcbf4aef DEV: Remove new_version_emails global setting reference
All site settings are now shadowed by global settings, so there is no need to lookup the global setting explicitly
2019-11-20 15:54:09 +00:00
Krzysztof Kotlarek 975165f25f
FIX: Don't error CleanUpInactiveUserJob when user is missing (#8362) 2019-11-18 16:14:15 +11:00
David Taylor 836b3f4d82
FIX: Do not deactivate admin accounts with recent posts or api keys (#8342)
This prevents 'bot' users being deactivated
2019-11-12 16:56:01 +00:00
Mark VanLandingham 3dd2f2f701
DEV: Remove RSS feed polling in favor of plugin (#8233) 2019-11-12 09:49:02 -06:00
David Taylor 52c5cf33f8
FEATURE: Overhaul of admin API key system (#8284)
- Allow revoking keys without deleting them
- Auto-revoke keys after a period of no use (default 6 months)
- Allow multiple keys per user
- Allow attaching a description to each key, for easier auditing
- Log changes to keys in the staff action log
- Move all key management to one place, and improve the UI
2019-11-05 14:10:23 +00:00
Krzysztof Kotlarek c32bd8ae48 FEATURE: Remove attachments and truncate raw field for incoming emails (#8253)
Adds the settings: 

raw_email_max_length, raw_rejected_email_max_length, delete_rejected_email_after_days. 

These settings control retention of the "raw" emails logs.

raw_email_max_length ensures that if we get incoming email that is huge we will truncate it removing uploads from the raw log.

raw_rejected_email_max_length introduces an even more aggressive truncation for rejected incoming mail. 

delete_rejected_email_after_days controls how many days we will keep rejected emails for (default 90)
2019-10-30 16:54:35 +11:00
Sam Saffron 3d85cc1e69 PERF: run expensive clean up uploads less frequently
Previously every hour we would run a full scan of the entire DB searching
for expired uploads that need to be moved to the tombstone folder.

This commit amends it so we only run the job 2 times per clean_orpha_uploads_grace_period_hours

There is a upper bound of 7 days so even if the grace period is set really
high it will still run at least once a week.

By default we have a 48 grace period so this amends it to run this cleanup
daily instead of hourly. This eliminates 23 times we run this ultra expensive
query.
2019-10-28 11:14:52 +11:00
Dan Ungureanu 1358312584
FIX: Zeitwerk-related fixes for jobs. (#8219) 2019-10-21 20:25:35 +03:00
Joffrey JAFFEUX b6ef473a31
DEV: prevents redefinition of various constants (#8199)
CLEANUP_GRACE_PERIOD
MAX_AWARDED
POLL_MAILBOX_TIMEOUT_ERROR_KEY
2019-10-17 09:18:07 +02:00
Dan Ungureanu 086b46051c
FIX: Zeitwerk-related fixes for jobs. (#8187) 2019-10-14 13:03:22 +03:00
Krzysztof Kotlarek 427d54b2b0 DEV: Upgrading Discourse to Zeitwerk (#8098)
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains. 

We no longer need to use Rails "require_dependency" anywhere and instead can just use standard 
Ruby patterns to require files.

This is a far reaching change and we expect some followups here.
2019-10-02 14:01:53 +10:00
Robin Ward d251f12c9c Tweak calculation for reviewable sensitivities/priorities
Previously, calculating thresholds for reviewables was done based on the
50th and 85th percentile across all reviewables. However, many forum
owners provided feedback that these thresholds were too easy to hit, in
particular when it came to auto hiding content.

The calculation has been adjusted to base the priorities on reviewables
that have a minimum of 2 scores (flags). This should push the amount of
flags required to hide something higher then before.
2019-09-19 14:07:56 -04:00
Robin Ward d5b52abf2f FIX: Require a min amount of reviewables before calculating thresholds
On forums with very few flags you don't want to calculate averages
because they won't be very useful. Stick with the defaults until we hit
15 reviewables at least.
2019-09-19 13:42:50 -04:00
Roman Rizzi 568232052e
DEV: Remove FlagQuery class and old code (#8064) 2019-09-12 13:21:33 -03:00
Gerhard Schlager 56c6e8e961 FIX: POP3 doesn't work with TLS 1.3 2019-09-11 18:43:19 +02:00
David Taylor e2449f9f23 Revert "Revert "Revert "FIX: Heartbeat check per sidekiq process (#7873)"""
This reverts commit c3497559be.
2019-08-30 11:26:16 +01:00
David Taylor 5d4fcc62bb Revert "FIX: heartbeat should be per host"
This reverts commit 17bed44405.
2019-08-30 11:25:59 +01:00
Sam Saffron 17bed44405 FIX: heartbeat should be per host
We need to schedule heartbeat on the correct host, otherwise some queues
may end up starving.
2019-08-30 16:10:08 +10:00
Sam Saffron c3497559be Revert "Revert "FIX: Heartbeat check per sidekiq process (#7873)""
This reverts commit e805d44965.
We now have mechanisms in place to ensure heartbeat will always
be scheduled even if the scheduler is overloaded per: 098f938b
2019-08-30 10:12:10 +10:00
Sam Saffron 4fce6484fe PERF: reduce window of consistency on user actions
Databases can have a lot of user actions, self joining and running an
aggregate on millions of rows can be very costly

This optimisation will reduce the regular window of consistency down to 13
hours, this ensures the job runs much faster
2019-08-29 13:27:04 +10:00
OsamaSayegh e805d44965 Revert "FIX: Heartbeat check per sidekiq process (#7873)"
This reverts commit 340855da55.
2019-08-27 11:56:23 +00:00
Osama Sayegh 340855da55
FIX: Heartbeat check per sidekiq process (#7873)
* FIX: Heartbeat check per sidekiq process

* Rename method

* Remove heartbeat queues of previous bootups

* Regis feedback

* Refactor before_start

* Update lib/demon/sidekiq.rb

Co-Authored-By: Régis Hanol <regis@hanol.fr>

* Update lib/demon/sidekiq.rb

Co-Authored-By: Régis Hanol <regis@hanol.fr>

* Expire redis keys after 3600 seconds

* Don't use redis to store the list of queues
2019-08-26 09:33:49 +03:00
Gerhard Schlager 00b75b4f4e FIX: Don't try to delete staged, unused admins and mods 2019-08-21 15:29:51 +02:00
Gerhard Schlager cc7b24b88b FIX: Regularly reset unknown extension of uploads
Discourse will try to detect the extension the next time the upload is needed. Maybe there was just a transient error the last time.
2019-08-21 10:23:20 +02:00
Bianca Nenciu f63b8bb79d FIX: Periodically ensure consistency of categories. (#7663) 2019-06-06 11:30:52 +02:00
Neil Lalonde ecc9c76692
FEATURE: dynamically update the topic heat settings monthly (#7670)
The site settings beginning with "topic views heat" and "topic post like
heat" are set to defaults when installing Discourse, but there has not
been a process or guidance for updating these values based on
community activity.

This feature will update them once a month. The low, medium, and
high settings will be based on the minimums of the 45th, 25th, and
10th percentile topics respectively, so that 45% of topics will have
some "heat".

Disable automatic changes with the automatic_topic_heat_values setting.
2019-06-04 10:34:07 -04:00
Sam Saffron 22abad4151 PERF: stop reindexing and skipping deleted posts 2019-06-04 17:53:35 +10:00
Sam Saffron 74c4f926fc FIX: drop deleted posts from search index
This does two things

1. Our "index grace period" has been wound down to 1 day, there is no point
keeping a bloated index for a week, usually when people delete stuff they
mean for it to be removed

2. We were never dropping deleted posts from the index, only posts from
deleted topics

These changes speed up search a tiny bit and reduce background work.
2019-06-04 17:19:59 +10:00
Sam Saffron 77300c1d8d DEV: reindex old data in a more consistent way
Previously we were grabbing arbitrary rows in many cases which makes
diagnosing issues in the indexer more complex
2019-06-04 11:47:24 +10:00
Robin Ward b85fa62882 FIX: Don't send replies to flags that are ignored by expiry
The `AutoQueueHandler` will ignore really old flags. In that case, don't
notify the user that the moderator is looking into it. They probably
never saw it because it didn't meet the reviewable minimum priority.
2019-05-27 17:24:36 -04:00
Robin Ward e74cd54fc6 REFACTOR: Replace score bonuses with low/med/high priorities
We removed score from the UX so it makes more sense to have sites set
priorities instead of score bonuses.
2019-05-23 11:54:45 -04:00
Robin Ward 225593d445 FIX: Small tweak to percentiles based on feedback 2019-05-16 13:45:36 -04:00
Gerhard Schlager bbab60fa9b FIX: Don’t try to delete inactive moderators
follow-up to 6d77156a
2019-05-13 21:44:03 +02:00
Guo Xiang Tan 8165ceb320 Make rubocop happy. 2019-05-13 09:55:44 +08:00
Sam Saffron 30990006a9 DEV: enable frozen string literal on all files
This reduces chances of errors where consumers of strings mutate inputs
and reduces memory usage of the app.

Test suite passes now, but there may be some stuff left, so we will run
a few sites on a branch prior to merging
2019-05-13 09:31:32 +08:00
Robin Ward 5af7c90bab FEATURE: Hide Reviewable scores, change score filter to Priority
We found score hard to understand. It is still there behind the scenes
for sorting purposes, but it is no longer shown.

You can now filter by minimum priority (low, med, high) instead of
score.
2019-05-07 14:05:23 -04:00
Guo Xiang Tan 152238b4cf DEV: Prefer `public_send` over `send`. 2019-05-07 09:33:21 +08:00
Sam Saffron 9be70a22cd DEV: introduce new API to look up dynamic site setting
This removes all uses of both `send` and `public_send` from consumers of
SiteSetting and instead introduces a `get` helper for dynamic lookup

This leads to much cleaner and safer code long term as we are always explicit
to test that a site setting is really there before sending an arbitrary
string to the class

It also removes a couple of risky stubs from the auth provider test
2019-05-07 11:00:30 +10:00
Sam Saffron f8eddd40ad PERF: remove avg_time calculations and regular jobs from posts and topics
After careful analysis of large data-sets it became apparent that avg_time
had no impact whatsoever on "best of" topic scoring. Calculating avg_time
was a very costly operation especially on large databases.

We have some longer term plans of introducing other weighting that is read
time based into our scoring for "best of" and "top" topics, but in the
interim to stop a large amount of work that is not achieving any value we
are removing the jobs.

Column removal will follow once we decide on a new replacement metric.
2019-05-06 15:59:01 +10:00
Guo Xiang Tan 24347ace10 FIX: Properly associate user_profiles background urls via upload id.
`Upload#url` is more likely and can change from time to time. When it
does changes, we don't want to have to look through multiple tables to
ensure that the URLs are all up to date. Instead, we simply associate
uploads properly to `UserProfile` so that it does not have to replicate
the URLs in the table.
2019-05-02 14:58:24 +08:00
Sam Saffron 8fc2b0124f PERF: destroy users in batches of 50 users
This speeds up the process as we no longer need to commit a transaction
per user deleted
2019-04-26 22:51:33 +10:00
Gerhard Schlager 6d77156a94 FIX: Don't try to delete inactive admins 2019-04-25 11:59:50 +02:00
Guo Xiang Tan e351b2ee26 Revert "FEATURE: stop running migrate to new scheme once done"
This reverts commit 2e346e6ea2.

Super seeded by 573ddcbbd0.
2019-04-24 14:03:31 +08:00
Sam Saffron 2e346e6ea2 FEATURE: stop running migrate to new scheme once done
This setting will automatically disable itself when there are no longer any
problem uploads
2019-04-24 14:32:39 +10:00
Guo Xiang Tan 149411ec90 PERF: Speed up `Upload.migrate_to_new_scheme` by limiting remap scope.
Doing a `LIKE` on `Post#raw` and `Post#cooked` takes forever on large
sites.
2019-04-24 11:56:48 +08:00
Robin Ward 6f56fba016 UX: Update post actions to "Approve Post" and "Reject Post"
This should be more clear.
2019-04-23 12:19:11 -04:00
Robin Ward 14f9d40e48 FEATURE: Clarify Reviewable User Actions
"Approve" is now "Approve User" and "Delete" is a dropdown with a choice
that allows you to block.
2019-04-17 11:44:17 -04:00
Dan Ungureanu e23c87c442
FIX: Prevent exception in rescue block.
> Job exception: undefined method `id' for nil:NilClass
> clean_up_inactive_users.rb:25:in `rescue in block in execute'
2019-04-16 01:27:39 +03:00
Robin Ward f7ebfb1acc FIX: Only consider pending queued posts for cleaning up uploads 2019-04-12 14:39:32 -04:00
Régis Hanol ab79060468 FIX: don't send activation reminder to staged users 2019-04-10 17:29:08 +02:00
Bianca Nenciu 3d545d66df FEATURE: Send user activation reminders. (#7280) 2019-04-10 16:53:52 +02:00
Tarek Khalil 442fb2facb FEATURE: Remove ignore feature SiteSetting and enable ignore by default (#7349) 2019-04-10 12:54:59 +02:00
Sam Saffron ec1c3559da PERF: correct clean up inactive so it does not clog scheduler
also add a hard limit of 1000 users per job run so we do not clog the
scheduler

destroyer.destroy has a transaction and this can have some serious complications
with the open record set find_each has going
2019-04-09 22:25:05 +10:00
Guo Xiang Tan 6d0e8821f8 FIX: Optimize query and avoid bloating memory in `Jobs::MigrateUploadScheme`. 2019-04-09 15:45:30 +08:00
Vinoth Kannan 914ada1c74 DEV: convert scheduled job EnsurePostUploadsExistence into a rake task 2019-04-09 02:07:35 +05:30
Guo Xiang Tan 108c231d1c FIX: Clean up `topic_search_data` of trashed topics.
This keeps the index and table smaller.
2019-04-08 16:53:39 +08:00
Guo Xiang Tan c4997ce85f FIX: Don't try and reindex posts that have been trashed. 2019-04-08 16:38:43 +08:00
Robin Ward c9ddc6ce30 FIX: Make sure the site setting works and fix build 2019-04-04 17:44:10 -04:00
Vinoth Kannan 2c7a50860f DEV: disable missing post uploads check by default. 2019-04-05 02:29:10 +05:30
Vinoth Kannan a385f6c48a REFACTOR: create custom field even when missing post uploads not found 2019-04-04 21:49:44 +05:30
Vinoth Kannan 44e87372da FEATURE: Add scheduled job to ensure s3 uploads existence 2019-04-04 20:37:44 +05:30
Robin Ward 588f61f717 FIX: Linting 2019-04-03 18:08:07 -04:00
Vinoth Kannan 3b581de3b8 remove unrelated code
already added in a separate job
2019-04-04 02:26:28 +05:30
Vinoth Kannan b3fb0a7039 FEATURE: ensure consistency of post uploads in cooked content 2019-04-04 02:23:28 +05:30
Robin Ward c1ea63bdc1 FIX: Reviewables should not be created for users until they are active
Conversely, if a user is deactivated the reviewable should automatically
be rejected.

Before this fix, if a user was not active they'd still show in the
review queue but without an "Approve" button which was confusing.
2019-04-03 15:25:00 -04:00
Guo Xiang Tan d151425353
PERF: Delete search data of posts from trashed topics periodically. (#7302)
This keeps both the index and table smaller.
2019-04-03 10:10:41 +08:00
Guo Xiang Tan feb731bffd FIX: Regenerate optimized images instead of migrating from old scheme.
`OptimizedImage.migrate_to_new_scheme` was optimizing optimized images
which we don't need to do. Regnerating the optimized image is way easier.
2019-04-03 09:45:02 +08:00
Guo Xiang Tan d8704c11ca PERF: Better use of index when queueing a topci for search reindex.
Also move `Search::INDEX_VERSION` to `SearchIndexer` which is where the
version is actually being used.
2019-04-02 09:53:37 +08:00
Guo Xiang Tan aa2311a7b0 FIX: Don't reindex posts belonging to a deleted topic for search.
Posts belonging to a deleted topic can't be index for search so we need
to avoid loading those post ids.
2019-04-02 07:36:53 +08:00
Guo Xiang Tan 3fc5dbb045 FIX: Don't attempt to reindex posts that have an empty raw.
If the post ids keep loading, we might end up in a situations where
we're always loading the same post ids over and over again without
indexing anything new.

Follow up to daeda80ada.
2019-04-02 07:13:33 +08:00
Robin Ward 76669bb5a6 FIX: Don't refer to pending review items as flags
They could be queued posts or users, and the notice should reflect that
properly.
2019-04-01 14:46:56 -04:00
Guo Xiang Tan daeda80ada
FIX: Don't index posts with empty `Post#raw` for search. (#7263)
* DEV: Remove unnecessary join in `Jobs::ReindexSearch`.

* FIX: Don't index posts with empty `Post#raw` for search.
2019-04-01 10:06:27 +08:00
Tarek Khalil b1cb95fc23
FEATURE: Introduce ignore duration selection (#7266)
* FEATURE: Introducing new UI for tracking User's ignored or muted states
2019-03-29 10:14:53 +00:00
Robin Ward b58867b6e9 FEATURE: New 'Reviewable' model to make reviewable items generic
Includes support for flags, reviewable users and queued posts, with REST API
backwards compatibility.

Co-Authored-By: romanrizzi <romanalejandro@gmail.com>
Co-Authored-By: jjaffeux <j.jaffeux@gmail.com>
2019-03-28 12:45:10 -04:00
Guo Xiang Tan b58c965aad FIX: Destroy optimized image if attempting to migrate to new scheme fails. 2019-03-26 14:37:34 +08:00
Tarek Khalil fed2dd9148 FEATURE: Add scheduled job to purge expired ignored users (#7211) 2019-03-20 11:01:43 +01:00
Guo Xiang Tan 4020c87680 DEV: Refactor tests for `Jobs::CleanUpInactiveUsers`.
* Remove use of 0 in favor of `TrustLevel.levels[:newuser]`.
* Consolidate two tests into a single one.
* Test that disabling the feature works.
* Avoid loading full ActiveRecord object in test when we only need to
know the existence of the record.
2019-03-19 09:57:21 +08:00
Bianca Nenciu 2347661a74 FEATURE: Clean up inactive users. (#7172) 2019-03-18 16:25:15 +01:00
Tarek Khalil bd6d31c9ec
FEATURE: Add `IgnoredUsersSummary` daily job (#7144)
* FEATURE: Add `IgnoredUsersSummary` daily job

## Why?

This is part of the [Ability to ignore a user feature](https://meta.discourse.org/t/ability-to-ignore-a-user/110254/8).

We want to:

1. Send an automatic group PM that goes out to moderators
2. When {x} users have Ignored the same user, threshold defined by a site setting, default of 5
3. Only send this message every X days which is defined by another site setting
2019-03-14 22:51:43 +00:00
Guo Xiang Tan b0c8fdd7da FIX: Properly support defaults for upload site settings. 2019-03-13 16:36:57 +08:00
Dan Ungureanu 32bae48fd3 DEV: Use User#human? User#bot? (#7140) 2019-03-12 07:58:14 +08:00
Gerhard Schlager 1121514799 UX: Localize date format in "new user of the month" message 2019-03-06 21:58:25 +01:00
Robin Ward 78ddc82952 FIX: Respect min_flags_staff_visibility for new flags too
There was a situation where if:

* There were new flags to review that met the visibility threshold

AND

* There were old flags that *didn't* meet the threshold

THEN

a pending flags notification would be sent out. This fixes that case.
Staff should not be notified of flags if they do not meet the threshold
and are old.
2019-01-25 11:27:43 -05:00
Robin Ward 96b2585a91 REFACTOR: Remove unncessary stubs from pending flags reminder
They seem to be calculated fine by the application, and stubbing
makes the tests more brittle and prone to regression.
2019-01-24 13:45:58 -05:00
Robin Ward f32de88dfc FIX: Don't notify of pending flags if min_flags_staff_visibility not met 2019-01-22 11:01:18 -05:00
Guo Xiang Tan 95e3841974 FIX: Remove old reference and use `MiniScheduler::Stat`. 2019-01-18 16:36:11 +08:00
Sam 384135845b FEATURE: introduce ultra_low priority queue
This commit introduces an ultra low priority queue for post rebakes. This
way rebakes can never interfere with regular sidekiq processing for cases
where we perform a large scale rebake.

Additionally it allows Post.rebake_old to be run with rate_limiter: false
to avoid triggering the limiter when rebaking. This is handy for cases
where you want to just force the full rebake and not wait for it to trickle
2019-01-17 14:53:19 +11:00
Saurabh Patel b63b399799 DEV: remove uploaded_meta_id column from category (#6725)
* DEV: remove uploaded_meta_id column from category

* remove uploaded_meta part
2019-01-10 09:37:21 +08:00
Sam e08a3f719c FEATURE: push post rebake regular task to low priority queue
This allows us to run regular rebakes without starving the normal queue.

It additionally adds the ability to specify queue with `Jobs.enqueue` so
we can specifically queue a job with lower priority using the `queue` arg.
2019-01-09 08:57:20 +11:00
Sam f74ef71130 DEV: add transaction and active check to invalidate job
Follows up on 3b76f19668 this per @tgxworld a transaction makes sense here

Very minor
2018-12-13 14:59:56 +11:00