Transient errors in migration are ignored, silently corrupting
data, and the migration is incomplete and misses many sources of
uploads, which will lead to an incorrect expectation of independence
from the remote object storage after announcing that the migration
was successful, regardles of whether transient errors permanently
corrupted the data.
Remove this migration until such time as it is re-written to
follow the same pattern has the migration to s3, moving the
core logic out of the task.
This PR fixes a race condition with the IMAP notification code. In the `Email::Receiver` we call the `NewPostManager` to create the post and enqueue jobs and sends alerts via `PostAlerter`. However, if the post alerter reaches the `notify_pm_users` and the `group_notifying_via_smtp` method _before_ the incoming email is updated with the post and topic, we unnecessarily send a notification to the person who just posted. The result of this is that the IMAP syncer re-imports the email sent to the user about their own post, which looks like this in the group inbox:
To fix this, we skip the jobs enqueued by `NewPostManager` and only enqueue them with `PostJobsEnqueuer` manually _after_ the incoming email record has been updated with the post and topic.
Other improvements:
* Moved code to calculate email addresses from `IncomingEmail` records into the topic, with a group passed in, for easier testing and debugging. It is not the responsibility of the post alerter to figure this stuff out.
* Add shortcut methods on `IncomingEmail` to split or provide an empty array for to and cc addresses to avoid repetition.
Feature for `Must Approve Users` setup. When a user is rejected, a staff member can optionally set a reason for audit purposes. In addition, feedback email can be sent to the user.
Meta: https://meta.discourse.org/t/account-rejection-email/103112/8
This PR adds functionality for the IMAP sync code to detect if a UID that is missing from the mail group mailbox is in the Spam/Junk folder for the mail account, and if so delete the associated Discourse topic. This is identical to what we do for emails that are moved for Trash.
If an email is missing but not in Spam or Trash, then we mark the incoming email record with imap_missing: true. This may be used in future to further filter or identify these emails, and perhaps go hunting for them in the email account in bulk.
Note: This adds some code duplication because the trash and spam email detection and handling is very similar. I intend to do more refactors/improvements to the IMAP sync code in time because there is a lot of room for improvement.
Splits the `ToggleTopicClosed` job into two distinct `OpenTopic` and `CloseTopic` jobs to make the code clearer. The old job cannot be deleted yet because of outstanding sidekiq schedules, so a todo has been added to do so later this year.
Also replaced mentions of `topic_status_update` with `topic_timer` in some files, because the `topic_status_update` model is obsolete and replaced by topic timer.
Added some shortcut methods for checking if a topic is open/whether a user can change an open topic.
If the sliding window size is N seconds, then a moment at the Nth second
should be considered as the moment outside of the sliding window.
Otherwise, if the sliding window is already full, at the Nth second,
a new call wouldn't be allowed, but a time to wait before the next call
would be equal to zero, which is confusing.
In other words, the end of the time range shouldn't be included in the
sliding window.
Let's say we start at the second 0, and the sliding window size is 10
seconds. In the current version of rate limiter, this sliding window will
be considered as a time range [0, 10] (including the end of the range),
which actually is 11 seconds in length.
After this fix, the time range will be considered as [0, 10)
(excluding the end of the range), which is exactly 10 seconds in length.
It was a problem because during this operation only the first frame
is kept. This commit removes the alternative solution to check if a GIF
image is animated.
* DEV: TopicTrackingState calls should happen in the background
It was observed that calling TopicTrackingState on popular topics could result in a large number of calls to redis, resulting in slow response times when posting replies.
These calls should be moved to a background job.
* DEV: PostUpdateTopicTrackingState should execute on default queue
Fixes a rare race condition causing the `Imap::Sync` class to create an incoming email and associated post/topic, which then kicks off the PostAlerter to notify others in the PM about a reply in the topic, but for the OP which is not necessary (because the person emailing the IMAP inbox already knows about the OP). Basically, we should never be sending the group SMTP email for the first post in a topic.
Also in this PR:
* Custom attribute accessors for the to/from/cc addresses on `IncomingEmail`, to parse them from an array to a joined string so the logic for this is only in one place.
* Store extra detail against the `IncomingEmail` created in `GroupSmtpMailer`
* regex test Mail header Reply-To as string instead of Field, which fixes `warning: deprecated Object#=~ is called on Mail::Field; it always returns nil`
* Add DEBUG_IMAP to log all IMAP logs as warnings for easier debugging
* Changed the Rails logging to `ImapSyncLog` in the `GroupSmtpMailer`
* FIX: Inline onebox should use encoding from Content-Type header when present
* Use Regexp.last_match(1)
Signed-off-by: OsamaSayegh <asooomaasoooma90@gmail.com>
- Only initialize the S3Helper when needed
- Skip initializing the S3Helper for S3Store#cdn_url
- Allow cook_url to be passed a `local` hint to skip unnecessary checks
These are a few small tweaks that slightly improve performance.
- we omitted 1 query from the post guardian which could cause an N+1
- cook_url has been sped up a bit
- url helper avoids re-creating sets for no reason
Inventory on S3 always lagged, over the past few weeks we are noticing that
1 day of lag is not enough.
We are increasing this to 2, to ensure that we do not get false positive
reports.
Final sigma is not lower cased correctly in Ruby causing issues with routing.
This works around the issue by downcasing all usernames containing a sigma using JS.
You can now use `@me` to search for posts created by yourself, this is particularly handy if you have a long username.
`@me rainbow` will find all posts you created with the word rainbow.
Also cleans up test suite so it has no warnings.
This allows plugins to call `register_demon_process` with a Class inheriting from Demon::Base. The unicorn master process will take care of spawning, monitoring and restarting the process. This API should be used with extreme caution, but it is significantly cleaner than spawning processes/threads in an `after_initialize` block.
This commit also cleans up the demon spawning logging so that it uses the same format as unicorn worker logging. It also switches to the block form of `fork` to ensure that Demons exit after running, rather than returning execution to where the fork took place.
`/srv/status` routes should not be cached at all. Also, we want to
decouple the route from Redis which `AnonymouseCache` relies on. The
`/srv/status` should continue to return a success response even if Redis
is down.
This is an edge-case of 9fb3629. An admin could set the shared draft category to one where both TL2 and TL3 users have access but only give shared draft access to TL3 users. If something like this happens, we need to make sure that TL2 users won't be able to see them, and they won't be listed on latest.
Before this change, `SharedDrafts` were lazily created when a destination category was selected. We now create it alongside the topic and set the destination to the same shared draft category.
* FEATURE: Allow categroy group moderators to list/unlist topics
If enabled via SiteSettings, a user belonging to a group which has been granted category group moderator privileges should be able to list/unlist topics belonging to the appropraite category.
It used to insert block Oneboxes inside paragraphs which resulted in
invalid HTML. This needed an additional parsing for removal of empty
paragraphs and the resulting HTML could still be invalid.
This commit ensure that block Oneboxes are inserted correctly, by
splitting the paragraph containing the link and putting the block
between the two. Paragraphs left with nothing but whitespaces will
be removed.
Follow up to 7f3a30d79f.
* FIX: 'false' value was treated as a truthy value
For example, latest.json?no_subcategories=false used to have set
no_subcategories to the string value of 'false', which is not false.
* DEV: Remove dead code
* FIX: Redirect to /none under the right conditions
These conditions are:
- neither /all or /none present
- only for default filter
* FIX: Build correct topic list filter
/none was never added to the topic list filter
* FIX: Do not show count for subcategories if 'none' category
* FIX: preload_key must contain /none if no_subcategories
All the data we need for the `info` and `credentials` auth hash
are obtained via the user info API, not the JWT. Using and verifying
the JWT can fail due to clock skew, so let's skip it completely.
PR opened to fix the upstream issue at https://github.com/zquestz/omniauth-google-oauth2/pull/392
Previously it matched the behavior of standard ActiveRecord after_commit callbacks. They do not work well within `joinable: false` nested transactions. Now `DB.after_commit` callbacks will only be run when the outermost transaction has been committed.
Tests always run inside transactions, so this also introduces some logic to run callbacks once the test-wrapping transaction is reached.
* DEV: More robust processing of URLs
The previous `UrlHelper.encode_component(CGI.unescapeHTML(UrlHelper.unencode(uri))` method would naively process URLs, which could result in a badly formed response.
`Addressable::URI.normalized_encode(uri)` appears to deal with these edge-cases in a more robust way.
* DEV: onebox should use UrlHelper
* DEV: fix spec
* DEV: Escape output when rendering local links
You can let non-staff users use shared drafts by modifying the `shared_drafts_min_trust_level` site setting. These users must have access to the shared draft category.
* FEATURE: Allow Category Group Moderators to edit topic titles
Adds category group moderators to the topic guardian’s `can_edit` method.
The value of `can_edit` is returned by the topic view serializer, and this value determines whether the current user can edit the title/category/tags of the topic directly (which category group moderators could already do by editing the first post of a topic).
Note that the value of `can_edit` is now always returned by the topic view serializer (ie, for both true and false values) to cover the case where a topic is moved out of a category that a category group moderator has permissions on, so that when the topic is reloaded the UI picks up that `can_edit` is now false, and thus the edit icon should no longer be displayed.
* DEV: Add a comment explaining why `can_edit` is always returned
If a category and a sub-category have the same slug, adding a background to one of them will also show it on the other one. This was introduced in 8e3f667 to fix a discrepancy, which was later fixed in 214b4c3.
We can't use erb in ember-cli, and it seems the emoji groups rarely
change anyway. This commit migrates the ERB to pre-rendered javascript
that is updated via the `rake javascript:update_constants` task.
- frowning was using slighty_frowning
- slightly_frowning was using frowning
- grinning_face_with_smiling_eyes was not defined
- fronwing_face_with_open_mouth was not defined
* FEATURE: onebox for local categories
This commit adjusts the category onebox to look more like the category boxes do on the category page.
Co-authored-by: Jordan Vidrine <jordan@jordanvidrine.com>
Force pushing a commit to a theme repository used to break the updater,
because the system was not able to count the commits behind the old and
new version. This operation failed because a force push deleted the old
commits.
The user was prompted with a simple "500 server error" message.
When calculating whether the attached uploads went over the SiteSetting.email_total_attachment_size_limit_kb.kilobytes limit, we were using the original_upload for the calculation instead of the actually attached_upload, which will be smaller in most cases because it can be an optimized image.
This adds a new min_trust_level_to_allow_ignore site setting that enables admins to control the point at which a user is allowed to ignore other users.
After the search term is parsed for advanced search filters, the term may
become empty. Later, the same term will be passed to Discourse.route_for
which will raise an ArgumentError.
> URI(nil)
ArgumentError: bad argument (expected URI object or URI string)
Partial revert of b143412be4 (the refactoring is not reverted)
This broke a few things, so we need to investigate and make some changes before reinstating the error
Hopefully this will prevent fellow developers from spending a morning
trying to figure out why the jobs that are run after a post is created
are not finding the post in the database.
That's because if you're inside a transaction when creating a post, the jobs
will likely run before the transaction is being commited to the database.
There was a "warning" in the comments of the PostCreator class but raising an
exception should hopefully help catch those issue much faster :fingers_crossed:.
This is hard to test since all the tests are run in a transaction...
* FEATURE: display error if Oneboxing fails due to HTTP error
- display warning if onebox URL is unresolvable
- display warning if attributes are missing
* FEATURE: Use new Instagram oEmbed endpoint if access token is configured
Instagram requires an Access Token to access their oEmbed endpoint. The requirements (from https://developers.facebook.com/docs/instagram/oembed/) are as follows:
- a Facebook Developer account, which you can create at developers.facebook.com
- a registered Facebook app
- the oEmbed Product added to the app
- an Access Token
- The Facebook app must be in Live Mode
The generated Access Token, once added to SiteSetting.facebook_app_access_token, will be passed to onebox. Onebox can then use this token to access the oEmbed endpoint to generate a onebox for Instagram.
* DEV: update user agent string
* DEV: don’t do HEAD requests against news.yahoo.com
* DEV: Bump onebox version from 2.1.5 to 2.1.6
* DEV: Avoid re-reading templates
* DEV: Tweaks to onebox mustache templates
* DEV: simplified error message for missing onebox data
* Apply suggestions from code review
Co-authored-by: Gerhard Schlager <mail@gerhard-schlager.at>
This moves the library into our lib folder, and refactored it to more
modern Javascript. I've kept the MIT license at the top of the file.
Doing this allows us to import it as a library in Ember CLI and ditch
yet another global variable.
Previously thumbnails were only preloaded for queries using `TopicQuery#default_results`, which meant that requests for PM topic lists would lead to N+1 queries.
This commit moves the preloading into TopicList#load_topics, along with other similar preloads (e.g. plugin custom fields)
The direct call to `ActiveRecord::Associations::Preloader#preload` is necessary because `@topics` can be an array, not an `ActiveRecord::Relation`
Themes marked for auto update will be automatically updated when
Discourse is updated. This is triggered by discourse_docker or
docker_manager running Rake task 'themes:update'.
CookedPostProcessor replaces all large images with their optimized
versions, but for GIF images the optimized version is limited to first
frame only. This caused animations it cooked posts to require a click
to show up the lightbox and start playing.
Animated emojis were converted to static images. This commit moves the
responsability on site admins to optimize their animated emojis before
uploading them (gifsicle is no longer used).
Plugin client.en.yml and server.en.yml can now be client/server-(1-100).en.yml. 1 is the lowest priority, and 100 is the highest priority. This allows plugins to set their priority higher than other plugins, so that they can override eachothers' translations.
It used to simply say "title is invalid" without giving any hint what
the problem could be. This commit adds different errors messages for
all caps titles, low entropy titles or titles with very long words.
- Bump rails_failover for new per-backend callback feature
- If the master backend fails over, make all sites readonly. And vice-versa for fallback
- If a single backend fails over, make that individual site readonly. And vice-versa for fallback
- When a single backend fails, also check connection to the master backend
Previously, `/u/by-external/{id}` would only work for 'Discourse SSO' systems. This commit adds a new 'provider' parameter to the URL: `/u/by-external/{provider}/{id}`
This is compatible with all auth methods which have migrated to the 'ManagedAuthenticator' pattern. That includes all core providers, and also popular plugins such as discourse-oauth2-basic and discourse-openid-connect.
The new route is admin-only, since some authenticators use sensitive information like email addresses as the external id.
This commit adds an additional find_user_by_email hook to ManagedAuthenticator so that GitHub login can continue to support secondary email addresses
The github_user_infos table will be dropped in a follow-up commit.
This is the last core authenticator to be migrated to ManagedAuthenticator 🎉
PostDestroyer should accept the option to permanently destroy post from the database. In addition, when the first post is destroyed it destroys the whole topic.
Currently, that feature is limited to private messages and creator of the post. It will be used by discourse-encrypt to explode encrypted private messages.
When embedding secure images which have been oneboxed, we checked to see if the image's parent's parent had the class onebox-body. This was not always effective as if the image does not get resized/optimized then it does not have the aspect-image div wrapping it. This would cause the image to embed in the email but be huge.
This PR changes the check to see if any of the image's ancestors have the class onebox-body, or if the image has the onebox-avatar class to account for variations in HTML structure.
Per Google, sites are encouraged to upgrade from Universal Analytics v3 `analytics.js` to v4 `gtag.js` for Google Analytics tracking. We're giving admins the option to stay on the v3 API or migrate to v4. Admins can change the implementation they're using via the `ga_version` site setting. Eventually Google will deprecate v3, but our implementation gives admins the choice on what to use for now.
We chose this implementation to make the change less error prone, as many site admins are using custom events via the v3 UA API. With the site stetting defaulted to `v3_analytics`, site analytics won't break until the admin is ready to make the migration.
Additionally, in the v4 implementation, we do not enable automatic pageview tracking (on by default in the v4 API). Instead we rely on Discourse's page change API to report pageviews on transition to avoid double-tracking.
There are issues around displaying images on published pages when secure media is enabled. This PR temporarily makes it appear as if published pages are enabled if secure media is also enabled.
* FEATURE - allow category group moderators to delete topics
* Allow individual posts to be deleted
* DEV - refactor for new `can_moderate_topic?` method
Previous to this change our anonymous rate limits acted as a throttle.
New implementation means we now also consider rate limited requests towards
the limit.
This means that if an anonymous user is hammering the server it will not be
able to get any requests through until it subsides with traffic.
See #10794 for original context.
I did not mean to add invite to the BYPASS_TYPES for Email::Sender, it was supposed to be invite_password_instructions.
A site owner attempting to use both the email_subject site setting and translation overrides for normal post notification
email subjects would find themselves frusturated at the lack of template argument parity.
Make all the variables available for translation overrides by adding the subject variables to the custom interpolation keys list and applying them.
Reported at https://meta.discourse.org/t/customize-subject-format-for-standard-emails/20801/47?u=riking
This commit adds a site setting `auto_close_topics_create_linked_topic`
which when enabled works in conjunction with `auto_close_topics_post_count`
setting and creates a new linked topic for the topic just closed.
The auto-created new topic contains a link for all the previous topics
and the topic titles are appended with `(Part {n})`.
The setting is enabled by default.
We had an issue where onebox thumbnail was too large and thus was optimized, and we are using the image URLs in post to redact and re-embed, based on the sha1 in the URL. Optimized image URLs have extra stuff on the end like _99x99 so we were not parsing out the sha1 correctly. Another issue I found was for posts that have giant images, the original was being used to embed in the email and thus would basically never get included because it is huge.
For example the URL 787b17ea61_2_690x335.jpeg was not parsed correctly; we would end up with 787b17ea6140f4f022eb7f1509a692f2873cfe35_2_690x335.jpeg as the sha1 which would not find the image to re-embed that was already attached to the email.
This fix will use the first optimized image of the detected upload when we are redacting and then re-embedding to make sure we are not sending giant things in email. Also, I detect if it is a onebox thumbnail or the site icon and force appropriate sizes and styles.
We remove the slow mode composer message and provide better messages when rejecting new posts and edits. The client now validates if the user tries to post again immediately. Finally, we replaced the `hourglass-end` icon with the `hourglass-start` one.
This reverts commit e3de45359f.
We need to improve out strategy by adding a cache breaker with this change ... some assets on CDNs and clients may have incorrect CORS headers which can cause stuff to break.
There is a site setting reply_by_email_enabled which when combined with reply_by_email_address creates a Reply-To header in emails in the format "test+%{reply_key}@test.com" along with a PostReplyKey record, so when replying Discourse knows where to route the reply.
However this conflicts with the IMAP implementation. Since we are sending the email for a group via SMTP and from their actual email account, we want all replys to go to that email account as well so the IMAP sync job can pick them up and put them in the correct place. So if the group has IMAP enabled and configured, then the reply-to header will be correct.
This PR also makes a further fix to 64b0b50 by using the correct recipient user for the PostReplyKey record. If the post user is used we encounter this error:
if destination.user_id != user.id && !forwarded_reply_key?(destination, user)
raise ReplyUserNotMatchingError, "post_reply_key.user_id => #{destination.user_id.inspect}, user.id => #{user.id.inspect}"
end
This is because the user above is found from the from_address, but the destination which is the PostReplyKey is made by the post.user, which will be different people.
When plugin spec is evaluated for the first time, it took 30 seconds to run:
```
rm -rf tmp/* && LOAD_PLUGINS=1 be rspec ./plugins/discourse-solved/spec/requests/topics_controller_spec.rb
```
Applying sprocket patch in test environment solves that issue
On forums with a large amount of posts when a user had a bookmark in the topic, PostgreSQL was using an inefficient query plan to fetch the first post of the topic. When running this ActiveRecord query:
```
topic.posts.with_deleted.where(post_number: 1).first
```
The following query plan was produced:
```
Limit (cost=0.43..583.49 rows=1 width=891) (actual time=3850.515..3850.515 rows=1 loops=1)
-> Index Scan using posts_pkey on posts (cost=0.43..391231.51 rows=671 width=891) (actual time=3850.514..3850.514
rows=1 loops=1)
Filter: ((topic_id = 160918) AND (post_number = 1))
Rows Removed by Filter: 2274520
Planning time: 0.200 ms
Execution time: 3850.559 ms
(6 rows)
```
The issue here is the combination of ORDER BY and LIMIT causing the ineficcient Index Scan using posts_pkey on posts to be used. When we correct the AR call to this:
```
topic.posts.with_deleted.find_by(post_number: 1)
```
We end up with a query that still has a LIMIT but no ORDER BY, which in turn creates a much more efficient query plan:
```
Limit (cost=0.43..1.44 rows=1 width=891) (actual time=0.033..0.034 rows=1 loops=1)
-> Index Scan using index_posts_on_topic_id_and_post_number on posts (cost=0.43..678.82 rows=671 width=891) (actua
l time=0.033..0.033 rows=1 loops=1)
Index Cond: ((topic_id = 160918) AND (post_number = 1))
Planning time: 0.167 ms
Execution time: 0.072 ms
(5 rows)
```
This query plan uses the correct index, `Index Scan using index_posts_on_topic_id_and_post_number on posts`. Note that this is only a problem on forums with a larger amount of posts; tiny forums would not notice the difference. On large forums a query for a topic that takes 1s without a bookmark can take 8-30 seconds, and even end up with 502 errors from nginx.
* FEATURE - Add SiteSettings to control JPEG image quality
`recompress_original_jpg_quality` - the maximum quality of a newly
uploaded file.
`image_preview_jpg_quality` - the maximum quality of OptimizedImages
* FEATURE: allow category group moderators to edit posts
If the `enable_category_group_moderation` SiteSetting is enabled, posts should be editable by those belonging to the appropraite groups.
Previously if a onebox timed out we would not present the users in the log
with any information regarding the onebox. This makes it very difficult to
debug.
This adds url/topic/user in the debugging output.
`max-width: 50%; max-height: 400px;` is a good fallback, however, if width and height are given and are smaller than fallback - we should persist that smaller size.
Our Email::Sender class accepts an optional user argument, which is used to create a PostReplyKey record when present. This record is used to sub out the %{reply_key} placeholder in the Reply-To mail header, so if we do not pass in the user we get a broken Reply-To header.
This is especially problematic in the IMAP group SMTP situation, because these emails go to customers that we are replying to, and when they reply to us the email bounces! This fixes the issue by passing user to the Email::Sender when sending a group_smtp email but there is still more to do in another PR.
This Email::Sender optional user is a bit of a footgun IMO, especially because most of the time we use it there is a user we can source. I would like to do another PR for this after this one to make the parameter not optional, so we don't end up with these reply issues down the line again.
This was made adjustable to allow rolling back quickly if problems came
up. The new behaviour was made default in 93137066 and no problems with
this have been reported.
This consolidates logic used to match routes in ApiKey, UserApiKey and DefaultCurrentUserProvider. This reduces duplicated logic, and will allow UserApiKeysScope to easily re-use the parameter matching logic from ApiKeyScope
Adds a new slow mode for topics that are heating up. Users will have to wait for a period of time before being able to post again.
We store this interval inside the topics table and track the last time a user posted using the last_posted_at datetime in the TopicUser relation.
It pauses Sidekiq, clears Redis (namespaced to the current site), clears Sidekiq jobs for the current site, restores the database and unpauses Sidekiq. Previously it stayed paused until the end of the restore.
Redis is cleared because we don't want any old data lying around (e.g. old Sidekiq jobs). Most data in Redis is prefixed with the name of the multisite, but Sidekiq jobs in a multisite are all stored in the same keys. So, deleting those jobs requires a little bit more logic.
Dependency on gifsicle, allow_animated_avatars and allow_animated_thumbnails
site settings were all removed. Animated GIF images are still allowed, but
the generated optimized images are no longer animated for those (which were
used for avatars and thumbnails).
The added 'animated' is populated by extracting information using FastImage.
This field was used to selectively reoptimize old animations. This process
happens in the background.
Limit git log output to the first line of the commit message
when generating the list of commits in a release. Some commit messages
are including the names of all commits that were squashed, resulting
in duplicate and confusing lines in the release notes.
discourse-perspective-api was not successfully running tests via the
qunit:test rake task due to inconsistent naming between core and the
repo. As a result we no longer need the mapping in the plugin rake task, too.
Now that we have support for user-selectable color schemes, it makes sense
to simplify seeding and theme updates in the wizard.
We now:
- seed only one theme, named "Default" (previously "Light")
- seed a user-selectable Dark color scheme
- rename the "Themes" wizard step to "Colors"
- update the default theme's color scheme if a default is set
(a new theme is created if there is no default)
Per Google, sites are encouraged to upgrade from `analytics.js` to `gtag.js` for Google Analytics tracking. This commit updates core Discourse to use the new `gtag.js` API Google is asking sites to use. This API has feature parity with `analytics.js` but does not use trackers.
Creates a BabelHelper builder using a default list of plugins, to ensure the transpiled code is always using the same plugins instead of differents plugins in different cases.
When the server gets overloaded and lots of requests start queuing server
will attempt to shed load by returning 429 errors on background requests.
The client can flag a request as background by setting the header:
`Discourse-Background` to `true`
Out-of-the-box we shed load when the queue time goes above 0.5 seconds.
The only request we shed at the moment is the request to load up a new post
when someone posts to a topic.
We can extend this as we go with a more general pattern on the client.
Previous to this change, rate limiting would "break" the post stream which
would make suggested topics vanish and users would have to scroll the page
to see more posts in the topic.
Server needs this protection for cases where tons of clients are navigated
to a topic and a new post is made. This can lead to a self inflicted denial
of service if enough clients are viewing the topic.
Due to the internal security design of Discourse it is hard for a large
number of clients to share a channel where we would pass the full post body
via the message bus.
It also renames (and deprecates) triggerNewPostInStream to triggerNewPostsInStream
This allows us to load a batch of new posts cleanly, so the controller can
keep track of a backlog
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Previously, any errors in those files would e.g. blow up the update process in docker_manager.
Now it prints out an error and proceeds as if there was no compatibility file.
Includes:
* DEV: Extract setup_git_repo
* DEV: Use `Dir.mktmpdir`
* DEV: Default to `main` branch (The latest versions of git already do this, so to avoid problems do this by default)
* FIX: second factor cannot be enabled if SSO is enabled
If `enable_sso` setting is enabled then admin should not be able to
enable `enforce_second_factor` setting as that will lock users out.
Co-authored-by: Robin Ward <robin.ward@gmail.com>
DEV: Replace instances of Discourse.base_uri with Discourse.base_path
This is clearer because the base_uri is actually just a path prefix. This continues the work started in 555f467.
We can't use erb in Ember CLI (since it does not have Ruby) so this has
been ported to use our `javascript:update_constants` rake test instead.
Note we don't have to run this every time a notification type as it's
only used by fixtures to fill in some specific types we test against.
See https://meta.discourse.org/t/email-address-change-confirmation-email-not-sent-but-every-other-notification-emails-are/165358
In short: with disable emails set to non-staff, email address change confirmation emails (those sent to the new address) are not sent for staff or admin members.
This was happening because we were looking up the staff user with the to_address of the email, but the to address was the new email address because we are sending a confirm email change email, and thus the user could not be found. We didn't need to do this anyway because we are passing the user into the Email::Sender class anyway.
* PERF: avoid lookbehinds when indexing search
Previously we used a `EmailCook.url_regexp` this regex used lookbehinds
Unfortunately certain strings could lead to pathological behavior causing
CPU to skyrocket and regex replace to take a very very long time.
EmailCook still needs a fix, but it is less urgent cause it already splits
to single lines. That said we will correct that as well in a seperate PR.
New implementation is far more naive and relies on the extra spaces search
indexer inserts.
When that site setting is enabled, the category counts (new/unread)
include the subcategory definition topics, but the topics aren't included
in the list. This fixes that discrepancy.
This plugin is only useful for developers, however, making it core allows us to centralize any component modification in one commit.
This integration also adds a new site_setting: `styleguide_admin_only` which allows to enable a styleguide on a live site while restricting visibility to admins only.
By default, styleguide is disabled.
See https://meta.discourse.org/t/changing-a-users-email/164512 for additional context.
Previously when an admin user changed a user's email we assumed that they would need a password reset too because they likely did not have access to their account. This proved to be incorrect, as there are other reasons a user needs admin to change their email. This PR:
* Changes the admin change email for user flow so the user is sent an email to confirm the change
* We now record who the email change request was requested by
* If the requested by user is admin and not the user we note this in the email sent to the user
* We also make the confirm change email route open to anonymous users, so it can be clicked by the user even if they do not have access to their account. If there is a logged in user we make sure the confirmation matches the current user.
Allows site administrators to pick different fonts for headings in the wizard and in their site settings. Also correctly displays the header logos in wizard previews.
When enable_personal_messages was disabled, moderators could not see
the private messages for the "moderators" group. The link was displayed
on the client side, but the checks on the server side did not allow it.
This is where they should be as far as ember is concerned. Note this is
a huge commit and we should be really careful everything continues to
work properly.
UploadRecovery only worked on missing Upload records. Now it also works with existing ones that have an invalid_etag status.
The specs (first that test the S3 path) are a bit of stub-a-palooza, but that's how much this class interacts with the the outside world. 🤷♂️
Upload.secure_media_url? raised an exceptions when the URL was invalid,
which was a issue in some situations where secure media URLs must be
removed.
For example, sending digests used PrettyText.strip_secure_media,
which used Upload.secure_media_url? to replace secure media with
placeholders. If the URL was invalid, then an exception would be raised
and left unhandled.
Now instead in UrlHelper.rails_route_from_url we return nil if there is something wrong with the URL.
Co-authored-by: Bianca Nenciu <nenciu.bianca@gmail.com>
The download link on the lightbox for images was not downloading the image if the upload was marked secure, because the code in the upload controller route was not respecting the dl=1 param for force download.
This PR fixes this so the download link works for secure images as well as regular ligthboxed images.
See https://meta.discourse.org/t/changing-a-users-email/164512 for context.
When admin changes an email for a user, we were incorrectly sending the password reset email to the user's old address. Also the new email does not come into effect until the reset password process is done, so this PR adds some notes to the admin to make this clearer.
Use the names as provided by discourse-fonts and remove the
translated strings.
It also ensures that the selected font is present in case a font will
be removed in the future.
This is a little bit of refactoring. Core Discourse should have default promotion message for TL2.
In addition, when the Discobot plugin is enabled, the user is invited to advanced training
Since 9e4ed03, moderators can view groups with visibility level set to "Group owners, members and moderators".
This fixes an issue where moderators can see the group in /g but then get a 404 when clicking on individual groups.
To check if a post contains any embedded media, we look if the "image_sizes" attribute is present in the new post manager arguments.
We want to see one boxed links, but we only store the raw content of the post. To work around this, I extracted the onebox logic from the composer editor into a module.
With secure media and the UploadSecurity class, we need a nice way for plugins to register custom upload types that should be considered public and never secure.
This is intended for use by plugins which are building their own topic lists, and want to include PMs alongside regular topics (e.g. discourse-assign). It does not get used directly in core.
* DEV - versions of JS files written to a JS file to be included by load-script and appended as params to URLs
* Formatting
* Incorporate feedback from PR
* Update filename of public-js-versions
Editing a post didn't update the `post_uploads` right away. Instead it relied on the `CookedPostProcessor`. This can lead to an inconsistent state if uploads are added or removed during an edit and, for some reason, the `ProcessPost` job doesn't run (successfully). This inconsistency leads to missing uploads, because the newly added uploads appear to be unused and will be deleted by the `CleanUpUploads` job. In addition to that, uploads, which got removed during the edit, appear to be still in use and won't be deleted by the background job.
This commit ensures that the `post_uploads` are updated during the edit without relying on a background job.
In some cases Discourse admins may opt for sessions not to persist when a
browser is closed.
This is particularly useful in healthcare and education settings where
computers are shared among multiple workers.
By default `persistent_sessions` site setting is enabled, to opt out you
must disable the site setting.
After restoring a backup it takes up to 48 hours for uploads stored on S3 to appear in the S3 inventory. This change prevents alerts about missing uploads by preventing the EnsureS3UploadsExistence job from running in the first 48 hours after a restore. During the restore it deletes the count of missing uploads from the PluginStore, so that an alert isn't triggered by an old number.
We must guarantee that "rel=noopener" was set if "target=_blank" is present, which is not always the case for trusted users. Also, if the link contains the "nofollow" attribute, it has to have the "ugc" attribute as well.
In c6ceda8c, a bug was introduced where an admin searching for his own
private messages will actually end up searching through all private
messages on the site.
Follow-up to c6ceda8c4e
This PR introduces a few important changes to secure media redaction in emails. First of all, two new site settings have been introduced:
* `secure_media_allow_embed_images_in_emails`: If enabled we will embed secure images in emails instead of redacting them.
* `secure_media_max_email_embed_image_size_kb`: The cap to the size of the secure image we will embed, defaulting to 1mb, so the email does not become too big. Max is 10mb. Works in tandem with `email_total_attachment_size_limit_kb`.
`Email::Sender` will now attach images to the email based on these settings. The sender will also call `inline_secure_images` in `Email::Styles` after secure media is redacted and attachments are added to replace redaction messages with attached images. I went with attachment and `cid` URLs because base64 image support is _still_ flaky in email clients.
All redaction of secure media is now handled in `Email::Styles` and calls out to `PrettyText.strip_secure_media` to do the actual stripping and replacing with placeholders. `app/mailers/group_smtp_mailer.rb` and `app/mailers/user_notifications.rb` no longer do any stripping because they are earlier in the pipeline than `Email::Styles`.
Finally the redaction notice has been restyled and includes a link to the media that the user can click, which will show it to them if they have the necessary permissions.
![image](https://user-images.githubusercontent.com/920448/92341012-b9a2c380-f0ff-11ea-860e-b376b4528357.png)
- Lets child components extend color definitions
- Includes default theme color definitions
- Fails gracefully on color stylesheet SCSS errors
- Includes theme variables when extending colors
This PR ensures that new bookmarks cannot be created for deleted posts and topics, and also makes sure that if a bookmark was created and then the topic deleted that the show topic page does not error from trying to retrieve the bookmark reminder at.
Adding these classes to the stylesheet link elements in order to toggle dark/light schemes via this theme-component. Eventually this theme-component could possible be merged into core.
DEV: add plugin hooks for silence message parameters
Allows plugins to add, and update extra silence message params for custom
i18n vars
Allows plugins to override system messages via `message_title` and
`message_raw` parameters. We can later expose these params where necessary via event
hooks. Expose the parameter for the on user_silenced trigger.
* DEV: Switch our fast_xor gem for xorcist
We use the `xor` function as part of password hashing and we want to use
a faster version than the native ruby xor'ing feature so we use a gem
for this.
fast_xor has been abandoned, and xorcist fixed our initial holdup for
switching in https://github.com/fny/xorcist/issues/4
xorcist also has jruby support so we can remove our jruby fallback
logic.
* Move using statement inside of class
"en_US" doesn't contain most of the translations, so it falls back to "en". But that behavior stopped translation overrides to work for pluralized strings in "en_US", because it relies on existing translations. This fixes it by looking up the existing translation in all fallback locales.
After thinking about it, I worry that this will potentially leave a site
setting set when people hit ctrl-c ... feels a tiny bit risky, so leaving
it out.
This commit adds a new site setting "allowed_onebox_iframes". By default, all onebox iframes are allowed. When the list of domains is restricted, Onebox will automatically skip engines which require those domains, and use a fallback engine.
This is a follow-up for f51ccea028 because specs were failing on macOS. macOS uses bsdtar by default and the --transform option isn't supported by bsdtar. But the -s option is and it works the same.
Originally disabled in 0c0192e. Upload specs now use separate paths for each spec worker.
Fixes an issue in UploadRecovery#recover_from_local – it didn't take into account the testing infix (e.g. test_0) in the uploads/tombstone paths.
With the addition of `PostSearchData#private_message`, a partial
index consisting of only search data from regular posts can be created.
The partial index helps to speed up searches on large sites since PG
will not have to do an index scan on the entire search data index which
has shown to be a bottle neck.
- Introduces uploads:delete_missing_s3 which can be used to "give up" and
delete broken records from the database
- Fixes a bug in fix_missing_s3 - crashing on deleted posts
- Adds more info to analyze_missing_s3
The filter noops if an incorrect username is passed. This filter is not
exposed as part of the UI but is only used when an admin transitions
from a search within a user's personal messages to the full page search.
Follow-up to 4b30799054.
Note the following query being generated where the filter for a user's
private messages is executed twice.
```sql
SELECT "posts"."id", "posts"."user_id", "posts"."topic_id", "posts"."post_number", "posts"."raw", "posts"."cooked", "posts"."created_at", "posts"."updated_at", "posts"."reply_to_post_number", "posts"."reply_count", "posts"."quote_count", "posts"."deleted_at", "posts"."off_topic_count", "posts"."like_count", "posts"."incoming_link_count", "posts"."bookmark_count", "posts"."score", "posts"."reads", "posts"."post_type", "posts"."sort_order", "posts"."last_editor_id", "posts"."hidden", "posts"."hidden_reason_id", "posts"."notify_moderators_count", "posts"."spam_count", "posts"."illegal_count", "posts"."inappropriate_count", "posts"."last_version_at", "posts"."user_deleted", "posts"."reply_to_user_id", "posts"."percent_rank", "posts"."notify_user_count", "posts"."like_score", "posts"."deleted_by_id", "posts"."edit_reason", "posts"."word_count", "posts"."version", "posts"."cook_method", "posts"."wiki", "posts"."baked_at", "posts"."baked_version", "posts"."hidden_at", "posts"."self_edits", "posts"."reply_quoted", "posts"."via_email", "posts"."raw_email", "posts"."public_version", "posts"."action_code", "posts"."locked_by_id", "posts"."image_upload_id", (TS_RANK_CD(
post_search_data.search_data,
TO_TSQUERY('english', '''test'':*ABCD'),
0|32
)
* (
CASE categories.search_priority
WHEN 2
THEN 0.6
WHEN 3
THEN 0.8
WHEN 4
THEN 1.2
WHEN 5
THEN 1.4
ELSE
CASE WHEN topics.closed
THEN 0.9
ELSE 1
END
END
)
) rank, topics.bumped_at topic_bumped_at FROM "posts" INNER JOIN "post_search_data" ON "post_search_data"."post_id" = "posts"."id" INNER JOIN "topics" ON "topics"."id" = "posts"."topic_id" AND ("topics"."deleted_at" IS NULL) LEFT JOIN categories ON categories.id = topics.category_id WHERE ("posts"."deleted_at" IS NULL) AND "posts"."post_type" IN (1, 2, 3) AND (topics.visible) AND (topics.archetype = 'private_message' AND post_search_data.private_message) AND (posts.topic_id IN (SELECT topic_id
FROM topic_allowed_users
WHERE user_id = 99999
UNION ALL
SELECT tg.topic_id
FROM topic_allowed_groups tg
JOIN group_users gu ON gu.user_id = 99999 AND gu.group_id = tg.group_id
)) AND (post_search_data.search_data @@ TO_TSQUERY('english', '''test'':*ABCD')) AND (posts.topic_id IN (SELECT topic_id
FROM topic_allowed_users
WHERE user_id = 99999
UNION ALL
SELECT tg.topic_id
FROM topic_allowed_groups tg
JOIN group_users gu ON gu.user_id = 99999 AND gu.group_id = tg.group_id
)) AND ((categories.id IS NULL) OR (NOT categories.read_restricted) OR (categories.id IN (999999))) ORDER BY rank DESC, topic_bumped_at DESC
```
Renamed from `private_messages` to `personal_messages` without
deprecation because the `private_messages` advanced search filter never
worked in the first place when it was implemented.
This also ensures that restoring a backup works when it was created with the wrong upload paths in the time between ab4c0a4970 (shortly after v2.6.0.beta1) and this fix.
Because we allow all the other flag types on a deleted post we should be
able to send a pm to the user letting them know why we deleted their
post.
Bug report:
https://meta.discourse.org/t/-/161156
Like "default watching" and "default tracking" categories option now the "regular" categories support is added. It will be useful for sites that are muted by default. The user option will be displayed only if `mute_all_categories_by_default` site setting is enabled.
* FIX: Unlike own posts on ownership transfer
If a user has liked a post that has passed the
`post_undo_action_window_mins` system setting window and you transfer ownership
of that post to that user you will be the owner of a post that you have
liked, but cannot unlike resulting in a weird UI behavior. This commit
fixes this issue.
The existing tests didn't check for the timeout window for unliking
posts so I added that in.
I couldn't find a good way to do this logic inside of the guardian class
so rather than duplicating behavior of the `PostActionDestroyer` class
inside of the `PostOwnerChanger` I decided to pass in a "bypass"
variable that could be used to check if the calling class is the
'post_owner_changer' and bypass the guardian instead. I went this route
because the guardian `can_delete_post_action` method has no way of
distinguishing how to allow a user to be able to unlike their own posts
after the timeout window but only on a post owner change.
* use an options hash instead
Enabling the moderators_manage_categories_and_groups site setting will allow moderator users to create/manage groups.
* show New Group form to moderators
* Allow moderators to update groups and read logs, where appropriate
* Rename site setting from create -> manage
* improved tests
* Migration should rename old log entries
* Log group changes, even if those changes mean you can no longer see the group
* Slight reshuffle
* RouteTo /g if they no longer have permissions to view group
Themes can now declare custom colors that get compiled in core's color definitions stylesheet, thus allowing themes to better support dark/light color schemes.
For example, if you need your theme to use tertiary for an element in a light color scheme and quaternary in a dark scheme, you can add the following SCSS to your theme's `color_definitions.scss` file:
```
:root {
--mytheme-tertiary-or-quaternary: #{dark-light-choose($tertiary, $quaternary)};
}
```
And then use the `--mytheme-tertiary-or-quaternary` variable as the color property of that element. You can also use this file to add color variables that use SCSS color transformation functions (lighten, darken, saturate, etc.) without compromising your theme's compatibility with different color schemes.
This avoids the samesite cookie related error on chrome. It also adds support for twitter 'GIF' content, and allows videos to resize smoothly for narrow devices.
Convert all IMAP logging to write to a database table for easier inspection. These logs are cleaned up daily if they are > 5 days old.
Logs can easily be watched in dev by setting DISCOURSE_DEV_LOG_LEVEL=\"debug\" and running tail -f development.log | grep IMAP
Previously we did an early return if either SiteSetting.tagging_enabled or SiteSetting.allow_staff_to_tag_pms was false when updating the email on the IMAP server -- however this also stopped us from archiving or deleting emails if either of these were disabled.
Add update for fetching git commits if they do not exist, eg with
clone --depth 1 - only can fetch via git fetch --depth 1 {remote} {ref}
the ref needs to be a full, non-ambiguous reference.
* If the error doesn't have a message, the class name will help
* example:
before: "Failed to download #{filename} because "
after: "Failed to download #{filename} because Aws::S3::Errors::NotFound"
This fixes an issue where a non-default theme set to use the base color
scheme (i.e. the theme had an empty `color_scheme_id`) was loading the
default theme's color scheme instead.
`rake uploads:analyze_missing` can be used get rich information regarding
uploads missing from s3 (where verified is false)
`rake uploads:fix_missing` is a work in progress task for automatically
correcting certain historic issues. At the moment it simply rebakes all
posts with missing uploads, but it will improve over time
Adds functionality to reflect topic delete in Discourse to IMAP inbox (Gmail only for now) and reflecting Gmail deletes in Discourse.
Adding lots of tests, various refactors and code improvements.
When Discourse topic is destroyed in PostDestroyer mark the topic incoming email as imap_sync: true, and do the opposite when post is recovered.
When we run the S3 inventory, mark uploads that exist as verified true, those that don't as verified false, and uploads not included in the check / not yet checked as verified nil.
The UI prevents users from trying to create tags on topics when they
don't have permission, but if you are trying to add tags to a topic via
the API and you don't have permission before this change it would
silently succeed in creating the topic, but it wouldn't have any tags.
Now a 422 error will be returned with an error message when trying to
create a topic with tags when tagging is disabled or you don't have
enough trust level to add tags to a topic.
Bug report: https://meta.discourse.org/t/-/70525/14
When a tab is open but left unattended for a while, the red, green, and blue
pills tend to go out of sync.
So whevener we open the notifications menu, we sync up the notification count
(eg. blue and green pills) with the server.
However, the reviewable count (eg. the red pill) is not a notification and
is located in the hamburger menu. This commit adds a new route on the server
side to retrieve the reviewable count for the current user and a ping
(refreshReviewableCount) from the client side to sync the reviewable count
whenever they open the hamburger menu.
REFACTOR: I also refactored the hamburger-menu widget code to prevent repetitive uses
of "this.".
PERF: I improved the performance of the 'notify_reviewable' job by doing only 1 query
to the database to retrieve all the pending reviewables and then tallying based on the
various rights.
Similar to `advanced_filter` I introduced `advanced_order`.
I needed a new option because default orders are evaluated after advanced_filter so I couldn't use it.
Also, that part is a little bit more generic
```
elsif word =~ /order:\w+/
@order = word.gsub('order:', '').to_sym
nil
```
After those changes, I can use them in plugins in this way:
```
Search.advanced_order(:votes) do |posts|
posts.reorder("COALESCE((SELECT dvvc.counter FROM discourse_voting_vote_counters dvvc WHERE dvvc.topic_id = subquery.topic_id), 0) DESC")
end
```
The poll breakdown modal replaces the grouped pie charts feature.
Includes:
* MODAL: Untangle `onSelectPanel`
Previously modal-tab component would call on click the onSelectPanel callback with itself (modal-tab) as `this` which severely limited its usefulness. Now showModal binds the callback to its controller.
"The PR includes a fix/change to d-modal (b7f6ec6) that hasn't been extracted to a separate PR because it's not currently possible to test a change like this in abstract, i.e. with dynamically created controllers/components in tests. The percentage/count toggle test for the poll breakdown feature is essentially a test for that d-modal modification."
If a plugin name contained the name of another plugin, the wrong stylesheets would be reloaded. For example, working on discourse-prometheus-alert-receiver would cause discourse-prometheus stylesheets to be reloaded.
In virtualdom, element 'properties' are not completely synonymous with element 'attributes'. In particular, `data-` properties will not be rendered as attributes. To ensure all attributes are passed through, we need to include them under an `attributes` key. For more info, see https://github.com/Matt-Esch/virtual-dom/blob/master/docs/vnode.md#custom-attributes-data-
This stops sync of tracking state when list is filtered, in the past this
would cause the tracking state to go off wack.
Additionally this introduces an alias for "filter=tracking", called "f=tracking"
This was done cause the term "filter" is used internally in 2 different ways
the main way is for /unread /new filtering.
Trying to also call a query param "filter" causes enormous amounts of
internal pain, this circumvents the issue.
In the near future, we will be swtiching to PG headlines to generate the
search blurb. As such, we need to replace audio and video links in the
raw data used for headline generation. This also means that we avoid
replacing links each time we need to generate the blurb.
This commit should cause no functional change
- Split into functions to avoid deep nesting
- Register custom field type, and remove manual json parse/serialize
- Recover from deleted upload records
Also adds a test to ensure pull_hotlinked_images redownloads secure images only once
The assign plugin is one of two situations where a post can be both a whisper and a small-action. Check the action_code field to filter out small-actions.
Running the reorder rake task was triggering the following error:
PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "post_timings_unique"
I re-worked the queries and refactored to use the same couple of queries for all similar tables/columns.
* Fixed an issue I introduced in the last PR where I am just archiving everything regardless of whether it is actually archived in Discourse man_facepalming
* Refactor group list_mailboxes IMAP code to use providers, add specs, and add provider code to get the correct prodivder
A first step to adding automatic dark mode color scheme switching. Adds a new SCSS file at `color_definitions.scss` that serves to output all SCSS color variables as CSS custom properties. And replaces all SCSS color variables with the new CSS custom properties throughout the stylesheets.
This is an alpha feature at this point, can only be enabled via console using the `default_dark_mode_color_scheme_id` site setting.
Adds a imap_group_id column to IncomingEmail to deal with an issue where we were trying to update emails in the mailbox, calling IncomingEmail.where(imap_sync: true). However UID and UIDVALIDITY could be the same across accounts. So if group A used IMAP details for Gmail account A, and group B used IMAP details for Gmail account B, and both tried to sync changes to an email with UID of 3 (e.g. changing Labels), one account could affect the other. This even applied to Archiving!
Also in this PR:
* Fix error occurring if we do a uid_fetch and no emails are returned
* Allow for creating labels within the target mailbox (previously we would not do this, only use existing labels)
* Improve consistency for log messages
* Add specs for generic IMAP provider (Gmail specs still to come)
* Add custom archiving support for Gmail
* Only use Message-ID for uniqueness of IncomingEmail if it was generated by us
* Various refactors and improvements
I added delete_when_reminder_sent to ignored_columns because it no longer exists and added a shortcut method delete_when_reminder_sent? to the Bookmark model. However I have been seeing some weird errors like:
> Job exception: unknown attribute 'delete_when_reminder_sent' for Bookmark.
So I am very suspicious. I am just renaming the method to auto_delete_when_reminder_sent? to avoid any potential conflicts.
Also found include_bookmark_delete_on_owner_reply? in PostSerializer which is used for nothing; I must have forgotten to delete it before.
For the following conditions, the TopicUser.bookmarked column was not updated correctly:
* When a bookmark was auto-deleted because the reminder was sent
* When a bookmark was auto-deleted because the owner of the bookmark replied to the topic
This adds another migration to fix the out-of-sync column and also some refactors to BookmarkManager to allow for more of these delete cases. BookmarkManager is used instead of directly destroying the bookmark in PostCreator and BookmarkReminderNotificationHandler.
This changes PG text search to only match the given title against
lexemes that are formed from the title. Likewise, the given raw will
only be matched against lexemes that are formed from the post's raw.
If a user posted a topic and Akismet decided it was spam, the topic gets deleted and put into the review queue. If a category moderator for that category marked the post/topic as "Not Spam" the topic did not get recovered correctly because Guardian.new(@user).can_review_topic?(@post.topic) returned false incorrectly because the topic was deleted.