There was an issue that occurred with this order of operations:
* An IMAP topic was created by emailing a group
* A second user was invited to the topic (not the OP and not the group)
* A user with access to the group replies to the topic
* The second user receives a user_private_message notification email because of their involvement in the topic
* The second user replies to the email via email
This new reply would then go and notify the other group PM users, except for those who emailed the group topic directly, which is handled via the group SMTP mailer. However because the new post already has an incoming email because it is parsed via the Email::Receiver via POP3 the group SMTP section of the post alerter is skipped, and the group's email address is not ignored for the user_private_message notification.
This PR fixes it so the group is not ever sent an email via the PM notification. This is important because any new emails in the group's IMAP inbox will be picked up by the Imap::Sync code and created as a new topic which is not at all desirable.
Also in this PR I split up the specs a bit more for group SMTP in the post alerter to make them easier to read and they each only test one thing.
As per @davidtaylorhq 's comment at 6e2be3e#r46069906, this fixes an oversight where if login_required is enabled and an anon user follows a confirm new email link they are forced to login, which is not what the intent of #10830 was.
* FEATURE: Give user menu icons alt attributes
This commit gives user menu icons + notifications alt attributes
* UI: Use Translations
This commit refactors the code injecting alt tags to the icons to use translations instead of hard-coded text.
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.
This adds a safe default to not process pop3 emails when the pop3 polling option is set up that are > 1 week old. This is to avoid the situation where an older mailbox is used, which causes us to go and process all emails in that mailbox, sending out error emails to the senders of emails which cannot be parsed successfully.
Admins can now edit translations in different languages without having to change their locale. We display a warning when there's a fallback language set.
When selecting the "date in post" option from the bookmark reminder modal, it was not converting the date from the post, which may be in a completely different timezone, to the current user's timezone.
This PR fixes it so the timezone from the post is used to parse the date then converted to the user's timezone.
Going from /latest?f=tracked to /latest will result in three
different topic list requests because the query params are not
considered when determining if the route is staying the same.
Fixes bug introduced by bd25627198
What happens is we send notifications to everyone involved in the group inbox topic about new posts, however we pass the param `skip_send_email_to: email_addresses`. In the above commit I removed the group email address from this `email_addresses` array. This breaks the IMAP inbox because we email the group with the reply, and the IMAP sync tool finds this email and opens a new unrelated topic with it.
This results in some fun disasters if allowed to happen. For now, just issue an oblique error message; a localized message will be added on the client.
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
It used to change the category of the topic, instead of the destination
category (topic.category_id instead of topic.shared_draft.category_id).
The shared drafts controls were displayed only if the current category
matched the 'shared drafts category', which was not true for shared
drafts that had their categories changed (affected by the previous bug).
Before this change we were setting the input after the query has been done, resulting in us overwriting the input if the user types during the query.
We don't need to update it after the query, we just need to ensure it's set when we load the page and then it should stay in sync.
If a group you're a member of is invited to a PM, you can no longer remove yourself from it. This means you won't be able to remove the message from your inbox, and even if you archive it, it'll come back once someone replies.
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
A while ago we made a change to display a warning after installing a theme component when the admin tries to leave the page without adding the new installed component to any themes (see 5e29ae3ef5).
However there is an edge case that we forgot to address, and that's when an admin installs a component and then immediately opens the install modal again to install another one which can result in the warning being shown twice at the same time.
This PR prevents that by showing the warning when opening the install modal if the conditions are met (new component and not added to any themes) instead of showing it after installing the second component.
Installing multiple copies of the same theme/component is possible, but you rarely need to actually have multiple copies installed. We've seen many times new admins installing duplicates of components because they were unaware it was already installed. This PR makes the theme installer modal loop through the existing themes when you click on 'install', and if there is a theme with a URL that matches the URL you entered, a warning will show up and you will need to click 'install' again to proceed.
When you type # or @ in the search box, a popup appears with
autocomplete suggestions. Currently, when the popup is rendered it has
the first item selected and upon pressing Enter, the first item is
inserted into the search box. The problem with this behavior is that the
first suggestion may not be what you want, and if you are typing quickly
and hit enter, the first suggestion (which is not what you want) is
inserted in the search box.
This PR amends the popup so that it has no suggestions selected by
default which means the enter key will not insert anything unless you
select a suggestion via the up or down arrow keys.
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`
- 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 2 indexes optimise performance on profile pages.
The summary page displays:
1. A list of "Top Link" - links sorted by number of clicks posted by user
2. A list of "Top Replies" - replies made by a user that go the most hearts
These two areas could devolve into full index or table scans, new indexes are there to avoid this cost on large dbs
One minor downside is that storage requirements go a tiny bit up to maintain the new indexes
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.
Googlebot handles no-index headers very elegantly. It advises to leave as many routes as possible open and uses headers for high fidelity rules regarding indexes.
Discourse adds special `x-robot-tags` noindex headers to users, badges, groups, search and tag routes.
Following up on b52143feff we now have it so Googlebot gets special handling.
Rest of the crawlers get a far more aggressive disallow list to protect against excessive crawling.
My initial implementation didn't consider this case. We should skip imported users if the "imported_id" field is present, even if there're other custom fields.
* DEV: Remove with_deleted workarounds for old Rails version
These workarounds using private APIs are no longer required in the latest version of Rails. The referenced issue (https://github.com/rails/rails/issues/4306) was closed in 2013. The acts_as_paranoid workaround which this was based on was removed for rails > 5.
Switching to using a scope also allows us to use it within a `belongs_to` relation (e.g. in the Poll model). This avoids issues which can be caused by unscoping all `where` clauses.
Predicates are not necessarily strings, so calling `.join(" AND ")` can sometimes cause weird errors. If we use `WhereClause#ast`, and then `.to_sql` we achieve the same thing with fully public APIs, and it will work successfully for all predicates.
The notification panel gets resized and the JS uses maxWidth of 320.
This tends to fight with the CSS causing notifications to "jump" a bit when a new one lands.
If we clear the in-process cache first, it might get re-filled from the
DB before we clear the DB cache. This would be more likely on high-traffic
sites.
Scrolling was not working as expected after clicking the browser back button and navigating back to the tag topic list. We need to wrap the scroll inside a debounce function to ensure that the "window.pageYOffset" property is populated before our function runs.
Include the enable_filtered_replies_view site setting in the admin UI
Adds title label to in-reply-to widget
Invokes the filtered UI when using replies_to_post_number as a query
parameter
Replaces the "Show All" button icon
Fixes grammar for "Viewing 1 reply to..." label
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.
chromium may report float device pixel ratio below 1.5 that is still clearly retina:
```
window.devicePixelRatio
1.4999998807907104
```
We used to round this down to 1 and not provide these browsers with retina avatars.
New algorithm is much more forgiving, anything over 1.1 gets 2x images, anything over 2.1 gets 3x images.
* 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
25563357 moved the logout redirect logic from the client-side to the server-side. Unfortunately the login_required check was lost during the refactoring which meant that non-login-required sites would redirect to `/login` after redirect, and immediately restart the login process. Depending on the SSO implementation, that can make it impossible for users to log out cleanly.
This commit restores the login_required check, and prevents the potential redirect loop.
Passes by_user to :user_unsilenced so plugins can detect whether or not
a silence was done automatically (by system user) or manually (by non-system)
Adds the ability to pass details in the action logger params so custom loggers
can pass their own details eg, in custom silence logs
We want to wrap the `Ember.run.debounce` function and internally call `Ember.run` instead when running tests.
This commit changes discourseDebounce to work the same way as `Ember.run.debounce`.
Now that `discourseDebounce` works exactly like `Ember.run.debounce`, let's replace it and only use `DiscourseDebounce` from now on.
Move debounce to discourse-common to be able to reuse it in different bundles
Keep old debounce file for backwards-compatibility
Safari overlays its own nav at the bottom 10% or so of the screen. This
makes buttons in that area virtually unclickable, so to ensure buttons
there are reachable, we need to add enough bottom padding to menu panels.
This is a tiny change that will allow users to hover the date element of a full page search result to see the raw date. It's not always easy to know what the exact date was "20d" ago, so hopefully this helps when it's relevant.
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 root cause of the issue was that the route was overriding the 'error' action from the correctly implemented handler in routes/application.js.
Remove the custom handler, and the duplicated template logic for displaying the errorHtml.
Fixes: e16b3da04a
Being that system badges ship with every instance of Discourse, we've opted to define the name, description, and long description in our locales files to promote translation into other languages. When an admin visited the overview page of a system badge in their admin panel, they were met with disabled inputs for these text properties. The problem is that we failed to educate the admin that the text needs to be managed via the site text customization settings.
This change adds a small "Customize Text" link under theses inputs that takes the admin to the specific site text customization where they can make desired changes.
* FIX: Restore dismissing the first notification
Reverts the temporary fix (8e4fea897e) and restores the feature introduced in e638d43f0a.
The issue that was the reason for the revert (https://meta.discourse.org/t/logins-redirects-to-missing-notifications-page/149718) was a combination of two bugs:
1. Fixed in this commit - the click listener was accidentally registered also for logged-out users. This meant that the first click on a page always trigger an AJAX call to the notifications endpoint (`/notifications?recent=true&limit=5`), which returned a 403 error. Now, this code is run only when the user is logged in.
2. A still unknown bug that I could not reproduce, which was somehow setting the login redirect cookie to the URL of that previously failed AJAX request.
When jobs are enqueued inside a transaction, it's possible that they will be executed before the necessary data is available in the database. This commit ensures all jobs are enqueued in an ActiveRecord after_commit hook.
One potential downside here is if the job fails to enqueue, the transaction will no longer be aborted. However, the chance of that happening is reasonably low, and the impact is significantly lower than the current issue where jobs are scheduled before their data is ready.
The propagated promise failure from model() caused the router to reject future route transitions, even though it correctly routed to the last-resort 404 page.
Co-authored-by: Jeff Wong <awole20@gmail.com>
We should always hide user_id in response when `hide_email_address_taken` setting is enabled. Currently, it can be used to determine if the email was used or not.
A small change that would allow components to extend the tag
display in the filter dropdown, like they can in other contexts.
Was requested in the tag icons component, see
https://meta.discourse.org/t/tag-icons-component/109757/60?u=pmusaraj
The PR also standardises tag styling in select-kit dropdowns.
This ensures that users are only served cached content in their own language. This commit also refactors to make use of the `Discourse.cache` framework rather than direct redis access
Notification is created by a job. If the job is evaluated before changes are committed to a database, a notification will have an incorrect URL.
Therefore, the job should be lodged in enqueue_jobs method which is triggered after the transaction:
```ruby
Topic.transaction do
move_posts_to topic
end
add_allowed_users(participants) if participants.present? && @move_to_pm
enqueue_jobs(topic)
```
I improved a little bit specs to ensure that the destination topic_id is set. However, that tests are passing even without code improvements. I couldn't find an easy way to "delay" database transaction.
Meta: https://meta.discourse.org/t/bug-with-notifications-for-moved-posts/168937
* 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
When the invite was being redeemed and the ReviewableUser record status
for the invited user was not pending an error was being raised.
This commit makes sure that we are only looking for ReviewableUser
record with status pending and updates that to approved.
* FIX: show/hide ignored users preferences
based on the current user trust level and the appropriate site setting.
* Allow us to await the `updateCurrentUser` call
Co-authored-by: Robin Ward <robin.ward@gmail.com>
User directory items are sorted by some activity metric. If those metrics have the same value, postgres does not guarantee the order in which they will be returned. This can cause issues in pagination - some users may appear twice, and some may be missed. To illustrate
```
pry(main)> query = DirectoryItem.where(period_type: DirectoryItem.period_types[:weekly]).order(:likes_received).limit(50);
pry(main)> page1 = query.offset(0).pluck(:id);
pry(main)> page2 = query.offset(50).pluck(:id);
pry(main)> (page1 & page2).count # users on both pages
=> 29
```
If we use the primary key to tie-break matching metrics, things are much more reliable
```
pry(main)> query = DirectoryItem.where(period_type: DirectoryItem.period_types[:weekly]).order(:likes_received, :id).limit(50);
pry(main)> page1 = query.offset(0).pluck(:id);
pry(main)> page2 = query.offset(50).pluck(:id);
pry(main)> (page1 & page2).count # users on both pages
=> 0
```
This most commonly effects new sites where all the directory metrics are zero.
The fact that the ordering is indeterminate makes it difficult to write a reliable test case for this.
If a user could not set tags because they had a trust level lower than
min_trust_level_to_tag_topics site setting, the "Create Topic" button
from a tag page would still show up and be enabled. Clicking it caused
the composer model to silently have the tags set.
Instead we use the inline `hbs` helper. Note in the non-Ember CLI
version this will not actually inline compile, but it will still work
for all our tests.
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
The list of SVG icons is unavailable in production, and the previous
refactor here was causing incorrect and noisy console warnings.
This also parses the `svgIconList` string in a dev environment, icons
should now match more accurately.
* fixed header/favicon's vertical alignment
* slightly increased header margin
* made the onebox padding symmetrical
* increased the right margin on small image elements
* removed extraneous pre bottom margin
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.
- Display reason for validation error when logging in via an authenticator
- Fix email validation handling for 'Discourse SSO', and add a spec
Previously, validation errors (e.g. blocked or already-taken emails) would raise a generic error with no useful information.
At the moment, when filtering by group, the directory will unconditionally return the current user at the top of the list. This is quite unexpected, given that the user is deliberately trying to filter the list. This commit makes sure the 'include current user' logic only triggers for unfiltered directories
* 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>
`setPermissionsGroups` would initialize an empty permissions object whenever new groups were added to the Tag Group. This meant that if you selected the `visible` permission and then added groups to the Tag Group, the `visible` permission would be obliterated and the Tag Group would be treated as though it was `private`.
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.
CSS are blocking resources, so keeping them below JS delays
rendering of the page. CSS should be loaded ASAP.
This change speeds up first contentful paint by 0.2s on localhost.
The slower the device, the bigger the difference could be.
