It used to check if an upload record exists, which is wrong because an
invalid upload record exists even if the upload was not created.
The other improvement is a better log message.
Previously watched words ignored topic titles when applying auto tagging rules.
Also copy has been improved to reflect how the system behaves.
The text hints that we are only watching first post now
scopes are incredibly annoying to preload, simply adding :user_emails is not
enough.
Instead of relying on scopes simply iterate through user_emails which is
properly preloaded.
This removes 2 * N+1 when generating user reports.
When posts are moved from one topic to another, the `topic_user.bookmarked` column for all users in the new and the old topic needs to be resynced, for example because a user bookmarks post 12 in topic 1, then it is moved to topic 2, the topic_user record for topic 1 should no longer be bookmarked. A background job has been added to sync the column for a specified topic, or for no topic at all, which does it for all topics like the migration.
Also includes a migration that we have run in the past to fix bad data.
----
This has been addressed in other places in the past:
https://github.com/discourse/discourse/pull/10211https://github.com/discourse/discourse/pull/10188
When bulk inviting, the uploaded CSV file may contain wrong values for
the user fields. This tries to automatically correct them by finding
the most similar option (by ignoring the case).
Admins can use bulk invites to pre-populate user fields. The imported
CSV file must have a header with "email" column (first position) and
names of the user fields (exact match).
Under the hood, the bulk invite will create staged users and populate
the user fields of those.
The user mailing list mode continued to be silently enabled and
UserEmail job checked just that ignoring site setting
disable_mailing_list_mode.
An additional migrate was added to set disable_mailing_list_mode
to false if any users enabled the mailing list mode already.
It was used both when inviting from a topic page and when creating
invites with "Send to topic on first login", while it should be used
only in the former case.
Onebox content may only be resolved during the process_post job. Onebox content could change the content of the excerpt, so we need to make sure the excerpt is updated accordingly.
The urls that we generate for mobile post notifications don't take into
account the subfolder url if a site happens to have one configured. When
this happens when you tap on a new mobile notification it takes you to
a url that doesn't work because it is missing the subfolder portion.
I honestly think this should be handled in the Post model like we do
with the Topic model. `Post.url` should know how to handle subfolder
installs, but that seemed like a very risky change because there are
lots of other places in the codebase where we tack on the base_path and
I didn't want to risk duplicating it.
I also found a small typo in the topics controller spec.
The user interface has been reorganized to show email and link invites
in the same screen. Staff has more control over creating and updating
invites. Bulk invite has also been improved with better explanations.
On the server side, many code paths for email and link invites have
been merged to avoid duplicated logic. The API returns better responses
with more appropriate HTTP status codes.
The bug was mentioned on meta https://meta.discourse.org/t/users-are-seeing-handling-of-unhandled-tag-again/155367
It was related to users who are watching a specific topic. In that case, when the hidden tag was added or removed to the topic they were notified by `NotifyTagChangeJob`.
That job should take hidden tags into consideration. If all changed tags are in a hidden group, it should exclude user not belong to that group.
At the same time, if visible to anyone tag is added or removed users watching topic should be notified.
The 'Discourse SSO' protocol is being rebranded to DiscourseConnect. This should help to reduce confusion when 'SSO' is used in the generic sense.
This commit aims to:
- Rename `sso_` site settings. DiscourseConnect specific ones are prefixed `discourse_connect_`. Generic settings are prefixed `auth_`
- Add (server-side-only) backwards compatibility for the old setting names, with deprecation notices
- Copy `site_settings` database records to the new names
- Rename relevant translation keys
- Update relevant translations
This commit does **not** aim to:
- Rename any Ruby classes or methods. This might be done in a future commit
- Change any URLs. This would break existing integrations
- Make any changes to the protocol. This would break existing integrations
- Change any functionality. Further normalization across DiscourseConnect and other auth methods will be done separately
The risks are:
- There is no backwards compatibility for site settings on the client-side. Accessing auth-related site settings in Javascript is fairly rare, and an error on the client side would not be security-critical.
- If a plugin is monkey-patching parts of the auth process, changes to locale keys could cause broken error messages. This should also be unlikely. The old site setting names remain functional, so security-related overrides will remain working.
A follow-up commit will be made with a post-deploy migration to delete the old `site_settings` rows.
This PR allows entering a float value for topic timers e.g. 0.5 for 30 minutes when entering hours, 0.5 for 12 hours when entering days. This is achieved by adding a new column to store the duration of a topic timer in minutes instead of the ambiguous both hours and days that it could be before.
This PR has ommitted the post migration to delete the duration column in topic timers; it will be done in a subsequent PR to ensure that no data is lost if the UPDATE query to set duration_mintues fails.
I have to keep the old keyword of duration in set_or_create_topic_timer for backwards compat, will remove at a later date after plugins are updated.
This PR revamps the topic timer UI, using the time shortcut selector from the bookmark modal.
* Fixes an issue where the duration of hours/days after last reply or auto delete replies was not enforced to be > 0
* Fixed an issue where the timer dropdown options were not reloaded correctly if the topic status changes in the background (use `MessageBus` to publish topic state in the open/close timer jobs)
* Moved the duration input and the "based on last post" option from the `future-date-input` component, as it was only used for topic timers. Also moved out the notice that is displayed which was also only relevant for topic timers.
This should make it easier to track down how the incoming email was created, which is one of four locations:
The POP3 poller (which picks up reply via email replies)
The admin email controller #handle_mail (which is where hosted mail is sent)
The IMAP sync tool
The group SMTP mailer, which sends emails when replying to IMAP topics, pre-emptively creating IncomingEmail records to avoid double syncing
Moves the topic timer jobs from being scheduled ahead of time with enqueue_at to a 5 minute scheduled run like bookmark reminders, in a new job called Jobs::EnqueueTopicTimers. Backwards compatibility is maintained by checking if an existing topic timer job is enqueued in sidekiq for the timer, and if it is not running it inside the new job.
The functionality to close/open a topic if it is in the opposite state still remains in the after_save block of TopicTimer, with further commentary, which is used for Open/Close Temporarily.
This also removes the ensure_consistency! functionality of topic timers as it is no longer needed; the new job will always pick up the timers because they are not stored in a fragile state of sidekiq.
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
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.
* 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`
osts from topics with 'auto delete replies timer' with more than
skip_auto_delete_reply_likes likes will no longer be deleted. If 0,
all posts will be deleted.
This commit is dedicated to https://twitter.com/FiloSottile/status/1335666583126073354 for reminding me that like timestamps are valuable data.
Likes additionally include the topic_id and post_number of the acted post, to aid in analysis. Flag export does not include the disposition by staff.
The `enqueue_jobs` is not correctly post-processing the post since the
post is being created inside a transaction block. This commit explicitly
enqueues the job outside transaction block.
When the linked topic is created we'll not hardcode the topic title and
let onebox work its magic instead so that the title can be updated
automatically.
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.
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.
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.
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.
Previously, Jobs::EnqueueDigestEmails would enqueue a digest job for every user, even if there are no topics to send. The digest job would exit, no email would send, and last_emailed_at would not change. 30 minutes later, Jobs::EnqueueDigestEmails would run again and re-enqueue jobs for the same users.
120fa8ad introduced a temporary mitigation for this issue, by randomly selecting a subset of those users each time.
This commit adds a new `digest_attempted_at` column to the `user_stats` table. This column is updated every time a digest job completes for a user. Using this, we can avoid scheduling digest jobs for the same user every 30 minutes. This also removes the random user selection in 120fa8ad, and instead prioritizes users who had digests attempted the longest time ago.
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.
* FEATURE: Export the entire user profile as json, not just bio/website
* FEATURE: Add session log information to user export
Even though the columns are named 'auth_token' etc, the content is not actually usable to log into the forum with. Despite all that, it is still truncated for export, to avoid any 'token hash cracking' situations.
