An invalid draft is the draft of a topic with a short title or body.
The client does not save these, but it will ask the client if they want
to save it. Even if the answer is 'yes', the draft is discarded. This
commit skips Save button for small drafts.
During some authentication flows (e.g. external auth with validated emails), some fields on the signup form are readonly. Previously, they were rendered in a simple `<span>`, with no associated label. This commit makes them render in a disabled `<input>` field, so that the styling matches the rest of the form.
A subtle background is added to the disabled input to distinguish them from editable inputs.
When a post is created, the draft sequence is increased and then older
drafts are automatically executing a raw SQL query. This skipped the
Draft model callbacks and did not update user's draft count.
I fixed another problem related to a raw SQL query from Draft.cleanup!
method.
Long posts may have `cooked` fields that produce tsvectors longer than
the maximum size of 1MiB (1,048,576 bytes). This commit uses just the
first million characters of the scrubbed cooked text for indexing.
Reducing the size to exactly 1MB (1_048_576) is not sufficient because
sometimes the output tsvector may be longer than the input and this
gives us some breathing room.
The current behaviour was producing random tests failures which where consistently reproducible using `seed=32037592518471299633729129648744282271`
The cause of this error, is a previous test not giving any topicId or categoryId resulting in a cache key "undefined-undefined", just like a possibly previous test. Reseting cache between tests, seems the most straightforward and future proof solution
We shouldn't be checking if a user is allowed to do an action in the logger. We should be checking it just before we perform the action. In fact, guardians in the logger can make things even worse in case of a security bug. Let's say we forgot to check user's permissions before performing some action, but we still have a call to the guardian in the logger. In this case, a user would perform the action anyway, and this action wouldn't even be logged!
I've checked all cases and I confirm that we're safe to delete this calls from the logger.
I've added two calls to guardians in admin/user_controller. We didn't have security bugs there, because regular users can't access admin/... routes at all. But it's good to have calls to guardian in these methods anyway, neighboring methods have them.
Because the enable_s3_uploads setting may be false for
some sites but GlobalSetting.use_s3? is true, we need to
remove this additional check in uppy-upload. The hidden
enable_direct_s3_uploads setting is sufficient.
This adds a few different things to allow for direct S3 uploads using uppy. **These changes are still not the default.** There are hidden `enable_experimental_image_uploader` and `enable_direct_s3_uploads` settings that must be turned on for any of this code to be used, and even if they are turned on only the User Card Background for the user profile actually uses uppy-image-uploader.
A new `ExternalUploadStub` model and database table is introduced in this pull request. This is used to keep track of uploads that are uploaded to a temporary location in S3 with the direct to S3 code, and they are eventually deleted a) when the direct upload is completed and b) after a certain time period of not being used.
### Starting a direct S3 upload
When an S3 direct upload is initiated with uppy, we first request a presigned PUT URL from the new `generate-presigned-put` endpoint in `UploadsController`. This generates an S3 key in the `temp` folder inside the correct bucket path, along with any metadata from the clientside (e.g. the SHA1 checksum described below). This will also create an `ExternalUploadStub` and store the details of the temp object key and the file being uploaded.
Once the clientside has this URL, uppy will upload the file direct to S3 using the presigned URL. Once the upload is complete we go to the next stage.
### Completing a direct S3 upload
Once the upload to S3 is done we call the new `complete-external-upload` route with the unique identifier of the `ExternalUploadStub` created earlier. Only the user who made the stub can complete the external upload. One of two paths is followed via the `ExternalUploadManager`.
1. If the object in S3 is too large (currently 100mb defined by `ExternalUploadManager::DOWNLOAD_LIMIT`) we do not download and generate the SHA1 for that file. Instead we create the `Upload` record via `UploadCreator` and simply copy it to its final destination on S3 then delete the initial temp file. Several modifications to `UploadCreator` have been made to accommodate this.
2. If the object in S3 is small enough, we download it. When the temporary S3 file is downloaded, we compare the SHA1 checksum generated by the browser with the actual SHA1 checksum of the file generated by ruby. The browser SHA1 checksum is stored on the object in S3 with metadata, and is generated via the `UppyChecksum` plugin. Keep in mind that some browsers will not generate this due to compatibility or other issues.
We then follow the normal `UploadCreator` path with one exception. To cut down on having to re-upload the file again, if there are no changes (such as resizing etc) to the file in `UploadCreator` we follow the same copy + delete temp path that we do for files that are too large.
3. Finally we return the serialized upload record back to the client
There are several errors that could happen that are handled by `UploadsController` as well.
Also in this PR is some refactoring of `displayErrorForUpload` to handle both uppy and jquery file uploader errors.
This commit adds the number of drafts a user has next to the "Draft"
label in the user preferences menu and activity tab. The count is
updated via MessageBus when a draft is created or destroyed.
The cache_fullpath for the Stylesheet::Manager was the same for
every test runner in a parallel test environment, so when other
specs or other places e.g. the stylesheets_controller_spec ran
rm -rf Stylesheet::Manager.cache_fullpath this caused errors
for other specs running that went through the
Stylesheet::Manager::Builder#compile path, causing the error
```
Errno::ENOENT:
No such file or directory @ rb_sysopen
```
Also fixed the stylesheet_controller which was interpolating Rails.root + CACHE_PATH
itself instead of just using Stylesheet::Manager.cache_fullpath
```
The <(unknown):ember849>#canBulkSelect computed property was just overriden. This removes the computed property and replaces it with a plain value, and has been deprecated.
```
Follow-up to 43058db3ca
* DEV: Return 400 instead of 500 for invalid top period
This change will prevent a fatal 500 error when passing in an invalid
period param value to the `/top` route.
* Check if the method exists first
I couldn't get `ListController.respond_to?` to work, but was still able
to check if the method exists with
`ListController.action_methods.include?`. This way we can avoid relying
on the `NoMethodError` exception which may be raised during the course
of executing the method.
* Just check if the period param value is valid
* Use the new TopTopic.validate_period method
This PR contains only tests. These tests are from my old PR with refactoring of future-date-input-selector. That PR was closed because we had some changes in our planes about our time-pickers and additionally these tests were flaky.
Tests in this PR aren't flaky, since they use fake time moments in the future. Tests just document current behaviour of future-date-input-selector.
This commit fixes two bugs. The first one is that onPresenceChange was
called with invalid arguments and it did not register a callback. The
second bug is that it triggered the wrong visibilitychange event. The
function it tried to call does not exist in all versions of MessageBus.
It is safer to trigger an event instead because that exists in all
versions.
* Copy remove_member to new `leave` method
* Remove unneeded code from the leave method
* Rearrange the leave method
* Remove unneeded code from the remove_member method
* Add tests
* Implement on the client side
Clicking on an incomplete link to a topic (/t/ID or /t/SLUG) from
another post could replace current history entry or create two: one for
the incomplete URL and another one for the correct one. Going back was
either impossible or took the user to a redirect loop, redirected back
to /t/ID which redirected them again to /t/SLUG/ID.
Using an invalid value was allowed. This commit tries to automatically
fix the color by adding missing # symbol or will show an error to the
user if it is not possible and it is not a CSS color either.
* Copy the add_members method to the new join method
* Remove unneeded code from the join method
* Rearrange the join method
* Remove unneeded stuff from the add_members method
* Extract add_user_to_group method
* Implement of the client side
* Tests
* Doesn't inline users.uniq
* Return promise from join.then()
* Remove unnecessary begin and end
* Revert "Return promise from join.then()"
This reverts commit bda84d8d
* Remove variable already_in_group
This is confusing because you're running the tests on the older version
of Ember. Use `/tests` for Ember CLI, and `/qunit` when using Rails'
asset pipeline (but only if REALLY necessary!)
We have CSS animations which depend on the timeline/progress being
completely cleared when navigating from one topic directly to another.
This always worked because our loading component would clear the entire page
between topics but with our new experimental loading component the DOM was being
re-used.
This patch ensures that the timeline is removed completely from the DOM
if the topic changes.
Will show the last 6 seen users as filtering suggestions when typing @ in quick search. (Previously the user suggestion required a character after the @.)
This also adds a default limit of 6 to the user search query, previously the backend was returning 20 results but a maximum of 6 results was being shown anyway.
- inlines dasherize helper in sk
- uses an ajax helper to load wizard's ajax lib when in wizard
- amends wizard's ajax lib to work with string as first arg
- disabled loading spinner in wizard as it's not available
When configured, all topics in the category inherits the slow mode
duration from the category's default.
Note that currently there is no way to remove the slow mode from the
topics once it has been set.
When the Forever option is selected for suspending a user, the user is suspended for 1000 years. Without customizing the site’s text, this time period is displayed to the user in the suspension email that is sent to the user, and if the user attempts to log back into the site. Telling someone that they have been suspended for 1000 years seems likely to come across as a bad attempt at humour.
This PR special case messages when a user suspended or silenced forever.
We now use the group's full name in group SMTP emails, so we are dropping the via #{site_name}. If group owners still want this they can just change the full name of the group.
Configuring staged users to watch categories and tags is a way to sign
them up to get many emails. These emails may be unwanted and get marked
as spam, hurting the site's email deliverability.
Users can opt-in to email notifications by logging on to their
account and configuring their own preferences.
If staff need to be able to configure these preferences on behalf of
staged users, the "allow changing staged user tracking" site setting
can be enabled. Default is to not allow it.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Flips content_security_policy_frame_ancestors default to enabled, and
removes HTTP_REFERER checks on embed requests, as the new referer
privacy options made the check fragile.
There was a UI bug when submitting multiple files in the same batch. We
would remove the disabled status of the submit button after the previous
file was sucesfully uploaded and the next one was still mid
optimization.
Reported at https://meta.discourse.org/t/-/194841/15?u=falco
* FIX: Clear stale status of reloaded reviewables
Navigating away from and back to the reviewables reloaded Reviewable
records, but did not clear the "stale" attribute.
* FEATURE: Show user who last acted on reviewable
When a user acts on a reviewable, all other clients are notified and a
generic "reviewable was resolved by someone" notice was shown instead of
the buttons. There is no need to keep secret the username of the acting
user.
Replaces the autocomplete overlay for categories and usernames on the search input and adds suggestions as items in the search results instead. Also adds the same behaviour for @mentions as well as special `in: status: order:` keywords. See PR for more details.
The `GroupsController#members` endpoint accepts a `desc` parameter to determine how members are sorted, but it's been deprecated in favor of a boolean `asc` parameter. However, in the frontend, specifically the group membership requests page was not updated entirely to use the `asc` param and it still passes a `desc` param when changing how group requests are sorted.
This commit updates the `group-requests` Ember controller so it passes a boolean `asc` param and removes all references of `desc`. The controller view/template has already been updated to use `asc`:
207c3085fc/app/assets/javascripts/discourse/app/templates/group-requests.hbs (L15-L16)
There was a bug with changing timestamps using the topic wrench button. Under some circumstances, a topic was disappearing from the top of the latest tab after changing timestamps. Steps to reproduce:
- Choose a topic on the latest tab (the topic should be created some time ago, but has recent posts)
- Change topic timestamps (for example, move them one day forward):
- Go back to the latest tab and see that topic has disappeared.
This PR fixes this. We were setting topic.bumped_at to the timestamp user specified on the modal. This is incorrect. Instead, we should be setting topic.bumped_at to the created_at timestamp of the last regular (not a whisper and so on) post on the topic.
We have had reports of tabs freezing in Firefox, and reporting an error
in this line. I haven't been able to reproduce, but I suspect the
`forEach` loop is at the heart of the issue, so I have replaced it with
(hopefully) a safer call.
* More refactoring
* Do not reload stylesheets with unchanged filenames
* Select last matching stylesheet
* No need to return anything except a status code from the server
* Switch a badge state before sending a request and then switch it back in case of an error
Currently when bulk-awarding a badge that can be granted multiple times, users in the CSV file are granted the badge once no matter how many times they're listed in the file and only if they don't have the badge already.
This PR adds a new option to the Badge Bulk Award feature so that it's possible to grant users a badge even if they already have the badge and as many times as they appear in the CSV file.
A couple of weeks we made a change that skipped compressing assets used by the theme qunit page: https://github.com/discourse/discourse/pull/13619. This is a follow-up PR to stop the application helper from generating the assets for the theme qunit page with `.br` or `.gzip` extensions when a site uses S3 as a CDN.
Fixes two issues:
- ignores invalid XML in custom icon sprite SVG file (and outputs an error if sprite was uploaded via admin UI)
- clears SVG sprite cache when deleting an `icons-sprite` upload in a theme
Use a Map to hold the best link element for each Onebox HTML element.
Using an Object did not work as intended because Object can use only
Strings or Symbols as keys. Using HTML elements (representing oneboxes)
as keys most probably converted them to some generic string and sometimes
different Oneboxes were associated same key. It seems to be browser and
content dependent, without any clear indication of what is happening
internally.
This bug caused link counts to show only for the last Onebox because
the best link from the last Onebox was considered for all the other
Oneboxes.
This PR fixes a couple of issues related to group SMTP:
1. When running the group SMTP job, we were exiting early if the email was for the OP because of an IMAP race condition. However this causes issues when replying as a new topic for an existing SMTP topic, as the recipient does not get the OP email which can cause threading problems.
2. When sending emails for a new topic spun out like the issue in 1., we are not maintaining the original subject/topic title because that is based on the incoming email record, which we were not doing because the group SMTP email was never sent because of issue 1.
Size of headings increased proportionally with their nesting because
their size was relative to the parent element (used em). This commit
makes headings from posts use rem instead which are relative to the
root HTML element.
<h1><div><h1>test</h1></div></h1> looks the same as <h1>test</h1> now.
Both of the commits in this PR are meant to fix the problem of invalid
option being shown in the flair chooser. An invalid option can be shown
if at some point it was a valid one - a group with a flair that was
later changed by an admin and flair was removed. The other option an
invalid option can be selected is if the user had a primary group when
the migration ran and copied the same value to the flair_group_id
column.
* FIX: Set flair_group_id only if group has flair
Follow up to 4ba93aac66.
* FIX: Do not show invalid option in flair chooser
If selected flair group became unavailable because the flair was removed
then the option would still be selected and visible as an ID only.
Previously we would re-calculate topic_user.liked for all users who have ever viewed the source or destination topic. This can be very expensive on large sites. Instead, we can use the array of moved post ids to find which users are actually affected by the move, and restrict the update query to only check/update their records.
On an example site this reduced the `update_post_action_cache` time from ~27s to 300ms
Scanning for all possible invalid post_timing records in the destination topics can be a very expensive operation. The main aim is to avoid the data clashing with soon-to-be-moved posts, so we can reduce the scope of the query by targeting only rows which would actually cause a clash. post_timings has an index on (topic_id, post_number), so this is very fast.
On an example site, this reduced the query from ~6s to <10ms
This PR adds the first use of Uppy in our codebase, hidden behind a enable_experimental_image_uploader site setting. When the setting is enabled only the user card background uploader will use the new uppy-image-uploader component added in this PR.
I've introduced an UppyUpload mixin that has feature parity with the existing Upload mixin, and improves it slightly to deal with multiple/single file distinctions and validations better. For now, this just supports the XHRUpload plugin for uppy, which keeps our existing POST to /uploads.json.
* FEATURE: add penalty history when silencing a user
Display penalty history (last 6 months) when silencing/suspending a user
* FEATURE: allow default penalty values to be chosen
Adds a site setting that designates default penalty values in hours.
Silence/suspend modals will auto-fill in the default values, but otherwise
will still allow moderators to pick and overwrite values as normal.
First silence/suspend: first value
Second silence/suspend: second value
etc.
Penalty counts are forgiven at the same rate as tl3 promotion requirements do.
Co-authored-by: jjaffeux <j.jaffeux@gmail.com>
When the New tab and the Unread tab are empty we show educational messages with links to the preferences page. Both links lead to preferences/account page. In fact, settings that changes behaviour of the New and the Unread tab are on the preferences/notifications page. This PR makes links lead there.
When a staged user tried to redeem an invite, a different username was
suggested and manually typing the staged username failed because the
username was not available.
It looks like this regressed in #10432.
A user can create a group if they're an admin or if they're a mod and the "moderators_manage_categories_and_groups" setting is enabled, so it's safe to always set "can_admin_group" to true for new groups.
It will let us configure automatic membership, default title, and effects on create.
When secure media is enabled or when upload secure status
is updated, we also try and update the upload ACL. However
if the object storage provider does not implement this we
get an Aws::S3::Errors::NotImplemented error. This PR handles
this error so the update_secure_status method does not error
out and still returns whether the secure status changed.
This changes from providing a string literal for the #sub replacement, to providing a block.
Because the block is provided the match object, it is presumed to have already performed all necessary backreferences.
This avoids any replacement of backreferences in the message body.
User flair was given by user's primary group. This PR separates the
two, adds a new field to the user model for flair group ID and users
can select their flair from user preferences now.
There was an issue with the TopicTimerEnqueuer where any timer
that failed to enqueue_typed_job with an error would prevent
all other pending timers after the one that errored from running.
To mitigate this we just capture the error and log it (so we can
still fix it if needed for bug crushing) and proceed with the
rest of the timer enqueues.
The commit https://github.com/discourse/discourse/pull/13544 highlighted
this issue originally in hosted sites.
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
* FEATURE: Redirect logged in user to invite topic
Users who were already logged in and were given an invite link to a
topic used to see an error message saying that they already have an
account and cannot redeem the invite. This commit amends that behavior
and redirects the user directly to the topic, if they can see it.
* FEATURE: Add logged in user to invite groups
Users who were already logged in and were given an invite link to a
group used to see an error message saying that they already have an
account and cannot redeem the invite. This commit amends that behavior
and adds the user to the group.
This cookie is used to transmit notification read state to the server. It is always cleared by the server on the next page load, so there is no need for the expiry to be so long. This commit updates it to expire at the end of the session (the default), and replaces raw `document.cookie` usage with our `cookie` library.
This PR adds uppy to the project with a custom JS build and the shims needed to import it into our JS code. We need a custom build of Uppy because we do not use webpack for our JS modules/build. The only way to get what you want from Uppy is to use the webpack modules or to include the entire Uppy project including all plugins in a single JS file. This way we can just use the plugins we actually want. Future PRs will actually use Uppy!
Take 2 of https://github.com/discourse/discourse/pull/13466.
Fixes a few issues with the original PR:
- color definition stylesheet target now includes the theme id, to avoid themes set to use the default color scheme loading the same stylesheet
- changes the internal cache key for color definition stylesheet to reset the pre-existing cache
A more complex algorithm was used to achieve consensus between server
and client lists of notifications. This commit uses a different and
more simple approach that ignores order, but updates read status of
existing notifications and removes stale notifications.
And also move all the "top topics by period" routes to query string param.
/top/monthly => /top?period=monthly
/c/:slug/:id/l/top/monthly => /c/:slug/:id/l/top?period=monthly
/tag/:slug/l/top/daily => /tag/:slug/l/top?period=daily (new)
Users can invite people to topics from secured category, but they will
not be redirected to the topic after signing up unless they have the
permissions to view the topic. This commit shows a warning when invite
is saved if the topic is in a secured category and none of the invite
groups are allowed to see it.
Use the `sidekiq_retry_in` code from Jobs::UserEmail in group SMTP. Also we don't need to keep `seconds_to_delay` -- sidekiq uses the default delay calculation if you return 0 or nil from the block. See 3330df0ee3/lib/sidekiq/job_retry.rb (L216-L234) for sidekiq default retry delay logic.
I experimented with extracting this into a concern or a module, but `sidekiq_retry_in` is quite magic and it would not allow me to abstract away into a module that calls some method specificall in the child job class.
I would love to write tests for this, but it does not seem possible (not sure if its because of our test
setup) to write tests that test sidekiq's retry capability, and I am not sure if we should be anyway. Initial addition
to UserEmail did not test this functionality
d224966a0e
* UX: Remove background image after image has loaded
If an image has a `smallUpload`, that may be set as the `background-image` on the `img` element, and the `img` element set to use `lazy` loading. When the browser decides to load the `src` of the image element, it is rendered on top of the existing background image.
However, if the image proper has a transparent background, the background image may be partially visible through the transparent portions of the image.
This change creates an `onload` event that removes the background image when the image proper has completed loading.
Skip group SMTP email (and add log) if:
* topic is deleted
* post is deleted
* smtp has been disabled for the group
Skip without log if:
* enable_smtp site setting is false
* disable_emails site setting is yes
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
For other private messages we have the site setting
personal_email_time_window_seconds (default 20s) which allows
people to edit their post etc. before the email is sent.
This PR makes the Jobs::GroupSmtpEmail enqueuer in the
PostAlerter use the same delay.
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
Exposes to Ember CLI environment the feature provided in the production env by `lib/stylesheet/manager.rb:295`.
Fixes development env compatibility with discourse-color-scheme-toggle.
This PR changes the order of the topic timer options
into a more logical order when the topic is open/closed.
Also, we are now hiding the "Schedule Publishing" option
if the topic is not a private message or in a private category.
It does not make sense to schedule publishing to a different
category for a public topic.
* FIX: Detect decode failures earlier in image optimization pipeline
Follow up to 9b51b9b but also detects the bug earlier and backs off.
What iOS 15 is doing is returning all zeroes to `ctx.getImageData`,
so we don't have to wait until resize to detect the problem.
* Update app/assets/javascripts/discourse/app/lib/media-optimization-utils.js
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
We want to put the name of the trust level in to generated URLs, not the human-readable form.
i.e.:
`/admin/users/list/newuser`
rather than:
`/admin/users/list/new user`
When a topic is fully merged into another topic we close it and schedule it for deleting. But last time I changed this place I added a bug – when merging all posts in topic except the first one the topic was closing too.
If the OP is not merged into another topic, the original topic shouldn't be closed and marked for deletion. This PR fixes this.
We want to remove completely our custom modal for uploading files in composer and directly trigger the system file picker.
This PR makes it happen. The fix is pretty simple since we already weren't using our custom modal on mobile. We just need to start using the same hidden <input type="file"> that we already use on mobile.
It seems to be pretty tricky to test opening a system modal so I haven't added new tests. We already have other tests for file uploading though. We directly trigger jquery-File-Upload plugin hooks in those tests - 3dda926cb2/app/assets/javascripts/discourse/tests/acceptance/composer-attachment-test.js (L89).
Multiselect data can be saved but when all are removed then data are not cleared
Ajax function is removing an empty array from request data. In that case, we should change `[]` to `null`.
We need that empty values to properly empty data.
A post is rendered multiple times when it is being loaded. Sometimes,
not all information is available and the best link in the Onebox cannot
be found.
The error was:
```
↪ Unit | Model | topic::recover [✔]
↪ Unit | Utility | emoji::emojiUnescape [✔]
↪ Unit | Utility | pretty-text::quoting a quote [✔]
↪ Unit | Utility | click-track::routes to internal urlsUnhandled request in test environment: /forum/t/1234/recover (PUT)
Error: Unhandled request in test environment: /forum/t/1234/recover (PUT)
at Pretender.server.unhandledRequest (discourse/tests/setup-tests:173:15)
at Pretender.handleRequest (pretender:400:14)
at FakeRequest.send (pretender:169:21)
at Object.send (jquery:10100:10)
at Function.ajax (jquery:9683:15)
at performAjax (discourse/app/lib/ajax:174:19)
at eval (discourse/app/lib/ajax:183:11)
at invokeCallback (ember:63104:17)
at publish (ember:63087:9)
at eval (ember:57463:16)
[✘]
```
* DEV: Don't duplicate a function
The duration column has been ignored since the commit
4af77f1e38
for topic_timers, we use duration_minutes instead.
Also removing the duration key from Topic.set_or_create_timer. The only
plugin to use this was discourse-solved, which doesn't use it any
longer
since
c722b94a97
There are some hard limits in browser Canvas implementations, that will
throw a runtime exception when crossed. Since those limits are platform
dependent, the best we can do is catch it and back off from trying to
optimize a problematic file.
For example, a 60MB PNG can be processed fine by Chrome but Firefox will
fail trying to extract the ImageData from the CanvasRenderingContext2D
with NS_ERROR_FAILURE.
Also cleans up the media-optimization-utils and add post-resize size logs
The styling between the "Create Invite" and "Share Topic" modals is
shared. The margin that was used to organize inputs in a list is not
needed for the "Share Topic" modal.
We changed (https://github.com/discourse/discourse/pull/13407) behaviour of the topic level bookmark button recently. That PR made the button be opening the edit bookmark modal when there is only one bookmark on the topic instead of just removing that bookmark as it was before.
This PR fixes the next problems that weren't taken into account in the previous PR:
1. Everything should work fine even on very big topics when a bookmarked post is unloaded from the post stream. I've added code that loads the post we need and makes everything work as expected
2. When at least one bookmark on the topic has a reminder, we should always be showing the icon with a clock on the topic level bookmark button
3. We should show correct tooltips for the topic level bookmark button
We do not want to show the In Reply To section of the
group SMTP email template, it is similar to Context Posts
which we removed and is unnecessary.
This PR also removes the link to staged user profiles in
the email; their email addresses will just be converted
to regular mailto: links.
This PR makes several changes to the group SMTP email contents to make it look more like a support inbox message.
* Remove the context posts, they only add clutter to the email and replies
* Display email addresses of staged users instead of odd generated usernames
* Add a "please reply above this line" message to sent emails
This PR backtracks a fair bit on this one https://github.com/discourse/discourse/pull/13220/files.
Instead of sending the group SMTP email for each user via `UserNotifications`, we are changing to send only one email with the existing `Jobs::GroupSmtpEmail` job and `GroupSmtpMailer`. We are changing this job and mailer along with `PostAlerter` to make the first topic allowed user the `to_address` for the email and any other `topic_allowed_users` to be the CC address on the email. This is to cut down on emails sent via SMTP, which is subject to daily limits from providers such as Gmail. We log these details in the `EmailLog` table now.
In addition to this, we have changed `PostAlerter` to no longer rely on incoming email email addresses for sending the `GroupSmtpEmail` job. This was unreliable as a user's email could have changed in the meantime. Also it was a little overcomplicated to use the incoming email records -- it is far simpler to reason about to just use topic allowed users.
This also adds a fix to include cc_addresses in the EmailLog.addressed_to_user scope.
There is a big difference between regular watched words and regular
expressions and this has been confusing in the past. This notice adds
an explanation.
This commit also reorganizes the code of the test modal.
* FEATURE: Staff can receive pending user reminders more frequently.
We now express the "pending_users_reminder_delay" in minutes instead of hours so staff can have finer control over the delay.
We need to keep in mind that the reminders could still take up to 20 minutes, even when using a lower value. We send them from a scheduled job.
* Migrate to a new site setting for the reminders delay
We want to submit the flag modal on pressing CTRL + ENTER and CMD + ENTER.
Here's how our modals work:
Every modal can be dismissed by pressing ESC. This behaviour can be disabled for a specific modal if we need to.
Every modal can be submitted by pressing ENTER if the cursor wasn't on a text area or a form at the moment of pressing.
Now, the flag modal is actually a one big form and pressing ENTER doesn't submit it. I've added submitting by CTRL+ENTER but at first it was interfering with the basic modal submitting by ENTER. It's a pretty tricky thing to fix because we use the keyup event for submitting by ENTER and we need to use the keydown event for submitting with modifiers (because submitting by CMD+ENTER on Macs doesn't work with keyup).
Eventually, I fixed the problem just by adding a possibility to disable default submitting on ENTER (in the same way as we already have the possibility of disabling dismissing on ESC). Then I disabled default submitting for the flag form and implemented submitting by CTRL+ENTER and CMD+ENTER. This way everything is simple and robust. I did it only for the flag modal but it'll be easy and safe to add the same behaviour to another modal.
ATM it only implements server side of it, as my need is for automation purposes. However it should probably be added in the UI too as it's unexpected to have pinned_until and no bannered_until.
This PR improves navigation within lightboxes that contain multiple images for both touch and non-touch devices.
Currently, if a gallery contains multiple large images, and you click on the one currently displayed, two things happen.
1. we zoom in
2. we navigate to the next image
a0bbc346cb/app/assets/javascripts/discourse/app/lib/lightbox.js (L43-L49)
So, you get taken to the next image, and it shows zoomed in, even when the intention was to zoom in on the previous image.
Magnific popup has an option to disable image-click navigation in galleries. This PR toggles that on for non-touch devices.
The result is that if you click on an image in a gallery on a non-touch device, we zoom in on that image instead of navigating to the next one.
This has no impact on arrow/keyboard navigation.
Magnific popup also has an API when images change; we reset the zoom class when that happens. So, when you navigate to the next image, it won't be zoomed in.
For touch devices, clicking on the image will navigate to the next one without zooming in. Users can pinch-zoom if they want to see more details on touch devices.
I used jQuery for this because both Magnific popup and our implementation for this are based on jQuery. No point making a few lines use vanilla for this when the rest doesn't.
Add Members could also invite new users via emails, but that was a less
known fact. Splitting the previous modal into two more accessible
modals should make this feature more discoverable.
Badges that are awarded multiple times can be favorite and not favorite
at the same time. This caused few problems when users tried to favorite
them as they were counted multiple times or their state was incorrectly
displayed.
Effectively reverts 3ddc33b07c
Makes the failure states testable; see the uncommented test.
I don't think we're re-catching these errors anyway?
_update:_
We did in a single instance in discourse-code-review but it wasn't really intentional and I fixed it in https://github.com/discourse/discourse-code-review/pull/73
* pretender wasn't catching the request because it ran after this test finished
* restore wasn't needed, we do `sinon.restore()` after each test
The error was:
```
↪ Unit | Model | user::resolvedTimezone [✔]
↪ Unit | Utility | url::routeTo with prefixUnhandled request in test environment: /forum/u/chuck.json (PUT)
Error: Unhandled request in test environment: /forum/u/chuck.json (PUT)
at Pretender.server.unhandledRequest (discourse/tests/setup-tests:173:15)
at Pretender.handleRequest (pretender:400:14)
at FakeRequest.send (pretender:169:21)
at Object.send (jquery:10100:10)
at Function.ajax (jquery:9683:15)
at performAjax (discourse/app/lib/ajax:174:19)
at eval (discourse/app/lib/ajax:183:11)
at invokeCallback (ember:63104:17)
at publish (ember:63087:9)
at eval (ember:57463:16)
[✘]
```
A minimal reproduction:
`http://localhost:3001/qunit?seed=3&testId=da76996b&testId=e52a53e7`
This adds the following columns to EmailLog:
* cc_addresses
* cc_user_ids
* topic_id
* raw
This is to bring the EmailLog table closer in parity to
IncomingEmail so it can be better utilized for Group SMTP
and IMAP mailing.
The raw column contains the full content of the outbound email,
but _only_ if the new hidden site setting
enable_raw_outbound_email_logging is enabled. Most sites do not
need it, and it's mostly required for IMAP and SMTP sending.
In the next pull request, there will be a migration to backfill
topic_id on the EmailLog table, at which point we can remove the
topic fallback method on EmailLog.
The exception page is shown before Ember can actually figure out what the final destination URL we're going to is.
This means that the new page is not present in the history stack, so if we attempt to use the history stack to go back, we will actually navigate back by two steps.
By instead forcing a navigation to the current URL, we achieve the goal of going "back" with no history mucking.
Unfortunately, the actual URL that was attempted is not available. Additionally, this only works for the on-screen back button and not the browser back.
Additionally, several modernizations of the exception page code were made.
This was previously broken by 59ef48c0b9 (#11425, #11424).
Centralize the logic into the exception controller, which avoids the problematic bug and makes it easy to add additional detailed 404 pages in the future.
Sometimes oneboxes contain the same link multiple times and the link
count was shown for each of them. This commit adds link count only to
the most important link, that being either a heading or the header of
the onebox.
Before this change, calling `StyleSheet::Manager.stylesheet_details`
for the first time resulted in multiple queries to the database. This is
because the code was modelled in a way where each `Theme` was loaded
from the database one at a time.
This PR restructures the code such that it allows us to load all the
theme records in a single query. It also allows us to eager load the
required associations upfront. In order to achieve this, I removed the
support of loading multiple themes per request. It was initially added
to support user selectable theme components but the feature was never
completed and abandoned because it wasn't a feature that we thought was
worth building.
If a user posted a URL that appeared inside a Onebox, then the user
got a duplicate link notice. This was fixed by skipping those links in
Ruby.
If a user posted a URL that was Oneboxes and contained other links that
appeared in previous posts, then the user got a duplicate link notice.
This was fixed by skipping those links in JavaScript.
The generated regular expressions did not contain \b which matched
every text that contained the word, even if it was only a substring of
a word.
For example, if "art" was a watched word a post containing word
"artist" matched.
The dismiss new keyboard shortcut (x,r) has been broken since
7a79bd7da3. A fix was done and JS
tests were added in 006d52f32b
and b01e4738ab but the test was not
quite correct and so the bottom dismiss new button was not clicked.
This also fixes an issue with our keyboard shortcut click handling.
If multiple elements matched the selector they were all clicked. Now
we just click the first match.
If you click on a bookmark in the post stream you get an Edit Bookmark modal. This does not happen if you click the topic bookmark button.
We want to open the Edit modal too if there is only one bookmark on a topic (it doesn't matter on the first post or not). The other behaviour if there are > 1 bookmarks in the topic is to prompt the user to confirm delete of all the bookmarks in the topic. This behaviour will stay as-is.
I have done some refactoring in this PR, and still, there is a place for improvement. For example, we don't call post.deleteBookmark() method when deleting several bookmarks. I just don't want to refactor too much in one PR.
The first thing we needed here was an enum rather than a boolean to determine how a directory_column was created. Now we have `automatic`, `user_field` and `plugin` directory columns.
This plugin API is assuming that the plugin has added a migration to a column to the `directory_items` table.
This was created to be initially used by discourse-solved. PR with API usage - https://github.com/discourse/discourse-solved/pull/137/
Anonymizing a user changed their email address, destroyed all
associated InvitedUser records, but did not destroy the invites
associated to user's email.
Note that this commit will also disable daily grouping for datasets with more than 30 data points. This will also smartly do the grouping by month when grouping a full year.
Profiling showed that we were roughly 10% of a request time creating all
the ActiveRecord objects for categories in the `Site` model on a site with 61 categories.
Instead of querying for the categories each time based on which categories the user can see,
we can just preload all of the categories upfront and filter out the
categories that the user can not see.
Adds the last updated at and by SMTP/IMAP fields to the UI, we were already storing them in the DB. Also makes sure that `imap_mailbox_name` being changed makes the last_updated_at/by field update for IMAP.
When dismissing new topics for the Tracked filter, the dismiss was
limited to 30 topics which is the default per page count for TopicQuery.
This happened even if you specified which topic IDs you were
selectively dismissing. This PR fixes that bug, and also moves
the per_page_count into a DEFAULT_PER_PAGE_COUNT for the TopicQuery
so it can be stubbed in tests.
Also moves the unused stub_const method into the spec helpers
for cases like this; it is much better to handle this in one place
with an ensure. In a follow up PR I will clean up other specs that
do the same thing and make them use stub_const.
The `bootstrap.json` contains most preloaded information but some routes
provide extra information, such as invites.
This fixes the issue by having the preload request pass on the preloaded
data from the source page, which is then merged with the bootstrap's
preloaded data for the final HTML payload.
Steps to reproduce the bug:
- Create bookmarks for several posts on a topic
- Click the topic level bookmark button, it’ll open the modal that asks to confirm clearing all bookmarks from the topic
- Choose No
- Try to push the topic level bookmark button again - it won’t work
And it's fixed with this commit
This can happen when an avatar-flair component is rendered to an anonymous user on a login_required site (e.g. when they are redeeming an invite). The lack of group information was causing an error to be raised. With this commit, it now simple skips rendering the flair.
* Revert "DEV: skips three tests following cc1e73 (#13386)"
This reverts commit 2be201660a.
* FIX: Do not refresh post stream twice
This also improves the test suite and simulates a long running request
* FIX: Update local copy of raw
When we call Bookmark.cleanup! we want to make sure that
topic_user.bookmarked is updated for topics linked to the
bookmarks that were deleted. Also when PostDestroyer calls
destroy and recover. We have a job for this already --
SyncTopicUserBookmarked -- so we just utilize that.
Subclasses must call #delete_user_actions inside build_actions to support user deletion. The method adds a delete user bundle, which has a delete and a delete + block option. Every subclass is responsible for implementing these actions.
Next Week should mean next Monday, Next Month - the first day of the next month, and so on.
Also, we'll be using the name "Next Monday" instead of "Next Week" because it's easier to understand. No one can get confused by next Monday.
* DEV: skips two tests following cc1e73
Following the fix in cc1e73b8e4 we now refresh the whole stream which causes expected states of these tests to not exist anymore.
I'm skipping theses tests while we decide for a better fix.
Adds a new `smtp_group_id` column to `EmailLog` which is filled in if the mail `from_address` matches a group's `email_username`. This is for easier debugging, so we know which emails have been sent via group SMTP.
We previously only showed the link to the Email section
of group settings if both SMTP and IMAP were enabled for
a site, but this is not necessary now, only SMTP can be
enabled by itself so we should show the section if SMTP
is enabled.
Rendering an empty flair element with the css `background-image: url();` causes the browser to attempt an image request against the current document URL. Making duplicate requests for the document URL can cause some unusual race conditions, especially related to cookies. If this user-avatar-flair element was present on the site homepage (e.g. if categories+latest is the homepage), then it can prevent the signup flow from working correctly.
This commit updates the user-avatar-flair component to be a transparent wrapper around the avatar-flair component. If the user has no flair, no avatar-flair element will be rendered. This avoids the `background-image: url();` situation, and fixes the auth flow.
This commit also removes the duplicate avatar flair rendering from the `latest-topic-list-item` component. This wasn't particularly obvious, since the duplicate flairs were being rendered directly on top of each other.
The problem was happening in component integration tests on the rendering stage, sometimes the rendering would never finish.
Using time moments in the future when faking time solves the problem. Unfortunately, I don't know why exactly it helps. It was just a lucky guess after some hours I spent trying to figure out what's going on. But I've done a lot of testings, so looks like it really works. I'll be monitoring builds for some time after merging this anyway.
Unit tests seem to work alright with moments in the past. And we don't fake time in acceptance tests at the moment but I guess they would very likely be flaky with time moments from the past since they also do rendering.
I'm actually thinking of moving all fake time moments to the future (including moments in unit tests) to decrease the chances of flakiness. But I don't want to do everything in one PR, because I can accidentally introduce new flakiness.
A pretty easy way of picking time moments in the future for tests is to use the 2100 year. It has the same calendar as 2021. If a day is Monday in 2021 it's Monday in 2100 too.
Before this fix if your forum was set up with a subfolder and you
clicked on a link to a different subfolder it would not work. For
example:
subfolder: /cool
link is: /about-us
Previously it would try to resolve /about-us as /cool/about-us. With
this fix it redirects to /about-us correctly.
Editing a post that was just posted caused it to be reloaded and made a
request to the server. This had an additional side effect where the
model instances used by post stream and composer would be different and
changes did not propagate correctly.
Notifying about a tag change sometimes resulted in loading a large
number of users in memory just to perform an exclusion. This commit
prefers to do inclusion (i.e. instead of exclude users X, do include
users in groups Y) and does it in SQL to avoid fetching unnecessary
data that is later discarded.
When replying to a user_private_message email originating from
a group PM that does _not_ have a reply key (e.g. when replying
directly to the group's SMTP address), we were mistakenly linking
the new post created from the reply to the OP and the user who
created the topic, based on the first IncomingEmail message ID in
the topic, rather than using the correct reply to user and post number
that the user actually replied to.
We now use the In-Reply-To header to look up the corresponding EmailLog
record when the user who replied was sent a user_private_message email,
and use the post from that as the reply_to_user/post.
This also removes superfluous filtering of incoming_email records. After
already filtering by message_id and then addressed_to_user (which only
returns incoming emails where the to, from, or cc address includes any
of the user's emails), we were filtering again but in the ruby code for
the exact same conditions. After removing this all existing tests still
pass.
Previously due to "rowheader" role we would read out topic titles twice.
This adjusts it so we apply the heading role only to the topic link.
In turn this makes navigation through topic lists more accurate (h) only
lands you on topic links. It also reduces the amount of duplicate reading
NVDA does.
Before:
Topic title link new topic link support link b481 link 19h link 2 button...
After:
Topic title link
This reduces noise, up and down once you land on a topic link can give you
more context.
* UX: Improvements to reorder categories UX
Before, moving a category from, for example, position 25 to position 0 would result in switching the positions of the two categories at those positions.
Category A at position 0 would move to position 25, and Category B at position 25 would move to position 0.
Instead of switching positions, the reorder categories function should retain the order of categories except for the one being moved.
So, Category B at position 25 would still move to position 0, but Category A is merely bumped down to position 1.
This improves the UX because if a user *really* wants to switch the two categories, it results in one extra step. However in the other (what I think is normal) case, it saves the 24 other switches the user has to make to get Category A back to position 1 (you can imagine the user having to click the up arrow button repeatedly to return Category A to the top of the page). Now, imagine trying to do this with a site with 100s of categories. Yikes!
The UX improvement described above is what this commit accomplishes by redesigning the `move()` method of the reorder-categories controller. It adds some overhead to adjust the positions of all categories in between the origin and target positions, but in testing this is not noticible to the user. It's better for the computer to do extra work than the user.
* UX: Allow decimal input in reorder-categories for more precise positioning.
A common UX pattern when reordering a list of items is to allow a user to specify a target position as a decimal between two valid integer positions. The user is indicating they want the target list item to move in between the list items at the positions on either side of the target position.
For example, say there are three categories Category A at position 0, Category B at position 1, and Category C at position 3.
To move Category C in between Categories A and B, a user can now simply update Category C's position to 0.5.
The `ember_jquery` bundle contains production builds of Ember and jQuery
which doesn't work with tests. This commits introduces a new
`theme_qunit_vendor` bundle which is copy of the `vendor` bundle but
doesn't contain `ember_jquery`.
This commit is a partial revert of
409c8585e4
Previously, the `transformed.blah` shortcut could only be used in top-level hbs statements like {{transformed.blah}}. When attempting to use it in a sub-expression like `{{concat "hello" transformed.world}}`, it would raise a "transformed is not defined" error.
This commit updates the shortcut logic to make `transformed.blah` and `attrs.blah` work consistently in all hbs expressions.
Co-authored-by: Jordan Vidrine <jordan@jordanvidrine.com>
We don't want to show the draft checkmark in the composer when drafts are saved, as it’s a little bit distracting to see it keeps appearing and disappearing. Only in the case of error does it need to show anything, we will be showing a "drafts offline" warning as we did it before.
An important detail is that the warning was appearing and disappearing all the time too. Now, the warning won’t be flashing while a user is typing, it’ll be disappearing only when the draft was eventually saved.
I made a change in https://github.com/discourse/discourse/pull/13083/files to suppress re-throwing the error from popupAjaxError if isTesting() but that causes issues in other places instead. If I remove it I get this error in the group email test I added, so I am removing that test here too.
* DEV: replace swipe events to use translate rather than left/right
translate is better for animations. also use native css animations for opening
and closing.
* a11y: respect prefers reduced motion on mobile timeline
* DEV: reduce jquery usage
* DEV: add tests for menu swipe events
test is run in 50% zoom/transform which means offsets and x of touch events need to be halved
Refactor test window to use a transform rather than non-standard zoom property
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
A followup to e3b0abc and a replacement PR for #13298.
Fixes long topic titles wrapping to a separate line in the dropdown search results.
Also replaces divs that were incorrectly nested inside spans.
* FIX: Quoting Oneboxed content should exclude formatting
When a post is quoted that includes Oneboxed content, we should not include the formatting generated by the Onebox. Rather, we should attempt to collapse the link referenced by the Onebox to a single line text link.
* DEV: fix tests
When a group only has SMTP enabled and not IMAP, we do not
want to enqueue the :group_smtp_email job because using the group's
SMTP credentials for sending user_private_message emails is
handled by the UserNotifications class.
We do not want the :group_smtp_email job to be enqueued because
that uses a reply key instead of the group.email_username
for the reply-to address which is not what we want for SMTP
only, and also creates an IncomingEmail record to prevent IMAP
double syncing which we do not need either.
There is an open question about what happens when IMAP is
enabled after SMTP has been enabled for a while, and also questions
around whether we could do away with :group_smtp_email altogether
and handle everything via EmailLog and UserNotifications, adding
additional columns to the former and modifying the Imap::Sync
class to take this into account...a lot more further testing
for IMAP needs to be done to answer those questions.
For now, this fix should be sufficient to get the correct
reply-to address for user_private_response messages sent in
response to emails sent directly to the group's
email_username SMTP address.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Tag-chooser component expects an array of blocked tags, but was passed
a string instead. That made tag-chooser to not allow any tags that were
a substring of the current one.
In Ember CLI addons get put into the vendor bundle, as opposed to their
own bundle like we're doing in the Rails app. We never use pretty-text
without our vendor bundle so this should have no difference on
performance.
We need to keep the pretty-text bundle for server side cooking.
Rather than returning the size of the currently rendered image in the composer window (which is dependent on browser settings such as window size and zoom level), return the actual dimensions of the image file itself.
(Also see commit abac614492 which was an earlier attempt to fix this by excluding Oneboxed images entirely. That was reverted as the CSS selector didn’t work on all browsers.)
This PR changes the `UserNotification` class to send outbound `user_private_message` using the group's SMTP settings, but only if:
* The first allowed_group on the topic has SMTP configured and enabled
* SiteSetting.enable_smtp is true
* The group does not have IMAP enabled, if this is enabled the `GroupSMTPMailer` handles things
The email is sent using the group's `email_username` as both the `from` and `reply-to` address, so when the user replies from their email it will go through the group's SMTP inbox, which needs to have email forwarding set up to send the message on to a location (such as a hosted site email address like meta@discoursemail.com) where it can be POSTed into discourse's handle_mail route.
Also includes a fix to `EmailReceiver#group_incoming_emails_regex` to include the `group.email_username` so the group does not get a staged user created and invited to the topic (which was a problem for IMAP), as well as updating `Group.find_by_email` to find using the `email_username` as well for inbound emails with that as the TO address.
#### Note
This is safe to merge without impacting anyone seriously. If people had SMTP enabled for a group they would have IMAP enabled too currently, and that is a very small amount of users because IMAP is an alpha product, and also because the UserNotification change has a guard to make sure it is not used if IMAP is enabled for the group. The existing IMAP tests work, and I tested this functionality by manually POSTing replies to the SMTP address into my local discourse.
There will probably be more work needed on this, but it needs to be tested further in a real hosted environment to continue.
If we don't escape periods, they are interpreted as wildcards and it
becomes impossible to visit profiles of other users whose usernames
match. E.g., if your username was `a.c` and attempted to visit `abc`'s
profile, you would be incorrectly redirected to your own profile.
Setting a key/value pair in DistributedCache involves waiting on the
write to Redis to finish. In most cases, we don't need to wait on the
setting of the cache to finish. We just need to take our return value
and move on.
This allows us to do DISTINCT on the topic_id to remove
duplicates (e.g. in extensions to the report SQL), and
also introduces an additional_join_sql string to allow
extensions to JOIN additional tables.
* FIX: Ignore `allowlistgeneric` Onebox image sizes
The size of an image contained within the preview pane of a Composer window may vary depending on the configuration of the browser displaying the Composer (e.g., dimension of browser window, zoom level, etc.).
Presently, the dimensions of the images from the browser creating the post containing the Onebox will be used to render the Onebox to anyone who views the post. It is safer to let the backend figure out the dimensions of the images. Therefore, exclude `.onebox.allowlistedgeneric` images from the list of `image_sizes` sent to the backend.
* DEV: Replace jQuery selector with pure JS
* DEV: remove more jQuery
After editing a post, it is refreshed by two ways. One of them is
triggered by the client side which will route the client to the edited
post and force a reload this way. The other way is via Message Bus.
This commit ignores both of the ways and tries to update the post
immediately and then refresh the post stream.
It used to require SiteSetting.min_trust_level_to_allow_invite to
invite a user to a group, even if the user existed and the inviter was
a group owner.
Users who use encoded slugs on their sites sometimes run into 500 error when pasting a link to another topic in a post. The problem happens when generating a backward "reflection" link that would appear in a linked topic. Link URL restricted on the database level to 500 chars in length. At first glance, it should work since we have a restriction on topic title length.
But it doesn't work when a site uses encoded slugs, like here (take a look at the URL). The link to a topic, in this case, can be much longer than 500 characters.
By the way, an error happens only when generating a "reflection" link and doesn't happen with a direct link, we truncate that link. It works because, in this case, the original long link is still present in the post body and can be used for navigation. But we can't do the same for backward "reflection" links (without rewriting their implementation), the whole link must be saved to the database.
The simplest and cleanest solution will be just to remove the restriction on the database level. Abuse is impossible here since we are already protected by the restriction on topic title length. There aren’t performance benefits in using length-constrained columns in Postgres, in fact, length-constrained columns need a few extra CPU cycles to check the length when storing data.
When a topic is fully merged into another topic we close it and schedule its deleting. But, because of a bug, if the merged topic contains some moderator actions or small actions it won't be merged. This change fixes this problem.
An important note: in general, we don't want to close a topic after moving posts if it still contains some regular posts or whispers. But when we are moving posts to a private message we don't want the notice about it to be publicly visible. So we use whispers with action_code == 'split_topic' instead of small_actions in such cases and we should ignore this specific kind of whispers when decide if we should close the merged topic.
It was not clear that replace watched words can be used to replace text
with URLs. This introduces a new watched word type that makes it easier
to understand.
I merged this PR in yesterday, finally thinking this was done https://github.com/discourse/discourse/pull/12958 but then a wild performance regression occurred. These are the problem methods:
1aa20bd681/app/serializers/topic_tracking_state_serializer.rb (L13-L21)
Turns out date comparison is super expensive on the backend _as well as_ the frontend.
The fix was to just move the `treat_as_new_topic_start_date` into the SQL query rather than using the slower `UserOption#treat_as_new_topic_start_date` method in ruby. After this change, 1% of the total time is spent with the `created_in_new_period` comparison instead of ~20%.
----
History:
Original PR which had to be reverted **https://github.com/discourse/discourse/pull/12555**. See the description there for what this PR is achieving, plus below.
The issue with the original PR is addressed in 92ef54f402
If you went to the `x unread` link for a tag Chrome would freeze up and possibly crash, or eventually unfreeze after nearly 10 mins. Other routes for unread/new were similarly slow. From profiling the issue was the `sync` function of `topic-tracking-state.js`, which calls down to `isNew` which in turn calls `moment`, a change I had made in the PR above. The time it takes locally with ~1400 topics in the tracking state is 2.3 seconds.
To solve this issue, I have moved these calculations for "created in new period" and "unread not too old" into the tracking state serializer.
When I was looking at the profiler I also noticed this issue which was just compounding the problem. Every time we modify topic tracking state we recalculate the sidebar tracking/everything/tag counts. However this calls `forEachTracked` and `countTags` which can be quite expensive as they go through the whole tracking state (and were also calling the removed moment functions).
I added some logs and this was being called 30 times when navigating to a new /unread route because `sync` is being called from `build-topic-route` (one for each topic loaded due to pagination). So I just added a debounce here and it makes things even faster.
Finally, I changed topic tracking state to use a Map so our counts of the state keys is faster (Maps have .size whereas objects you have to do Object.keys(obj) which is O(n).)
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
Refactors `TrustLevel` and moves translations from server to client
Additional changes:
* "staff" and "admin" wasn't translatable in site settings
* it replaces a concatenated string with a translation
* uses translation for trust levels in users_by_trust_level report
* adds a DB migration to rename keys of translation overrides affected by this commit
In Ember CLI, the vendor bundler includes Ember/jQuery, so this brings
our app closer to that configuration.
We have a couple pages (Reset Password / Confirm New Email) where we need
`ember_jquery` without vendor so the file still exists for those cases.
Previous to this change we would switch off MessageBus updating after 20
minutes.
This ensures that when the user becomes present again we turn on long polling.
Without long polling updates can be delayed for minutes.
The widget should accept the disabled option.
In that case, CSS class "disabled".
In addition, after click dropdown will not be shown.
Also, the option to disable a specific value in a dropdown is included
…and just prioritize the current one, instead of hiding other categories.
Context: when you open the composer by clicking "New Topic" button when in a category, or by clicking "New Topic" in the share-popup, the category selector shows only the current category and its children (and "Uncategorized"). You can still find other categories, but you have to search by name.
This PR changes that, so you now can see all the categories in the dropdown, and those that are relevant (again: current, children and uncategorized) are displayed before all other categories.
tldr: don't make choosing other categories harder - make choosing relevant ones easier.
1. It defaults to `cache: true` already
2. Setting it to `false` for non-GET request doesn't do anything
3. We were correcting `cache: false` GET requests to use `cache: true`
…so setting it to anything at all, for any type of request doesn't make sense (anymore)
When the client received a new notification, it prioritized only PM
notifications instead of maintaining the priority order. Later, the
check for missing notification deleted all notifications that were
in the wrong order because it could not match the IDs.
The correct order puts high_priority AND unread notifications first.
Low priority or read notifications (including high priority, but read
notifications) come after.
Original PR which had to be reverted **https://github.com/discourse/discourse/pull/12555**. See the description there for what this PR is achieving, plus below.
The issue with the original PR is addressed in 92ef54f402
If you went to the `x unread` link for a tag Chrome would freeze up and possibly crash, or eventually unfreeze after nearly 10 mins. Other routes for unread/new were similarly slow. From profiling the issue was the `sync` function of `topic-tracking-state.js`, which calls down to `isNew` which in turn calls `moment`, a change I had made in the PR above. The time it takes locally with ~1400 topics in the tracking state is 2.3 seconds.
To solve this issue, I have moved these calculations for "created in new period" and "unread not too old" into the tracking state serializer.
When I was looking at the profiler I also noticed this issue which was just compounding the problem. Every time we modify topic tracking state we recalculate the sidebar tracking/everything/tag counts. However this calls `forEachTracked` and `countTags` which can be quite expensive as they go through the whole tracking state (and were also calling the removed moment functions).
I added some logs and this was being called 30 times when navigating to a new /unread route because `sync` is being called from `build-topic-route` (one for each topic loaded due to pagination). So I just added a debounce here and it makes things even faster.
Finally, I changed topic tracking state to use a Map so our counts of the state keys is faster (Maps have .size whereas objects you have to do Object.keys(obj) which is O(n).)
A non-staff user cannot post to a closed topic, so we should not
show them the modal asking "Which topic do you want to reply to?"
This also fixes an issue I ran into while testing the above change, in
Ember CLI an error was being raised because related messages were being
set inside a computed property.
When a topic is fully merged into another topic we close it. Now we want also to set a timer for deleting this topic. By default, stub topics will be deleted in 7 days. Users can change this period or disable auto-deleting by setting the period to 0.
This overhauls the user interface for the group email settings management, aiming to make it a lot easier to test the settings entered and confirm they are correct before proceeding. We do this by forcing the user to test the settings before they can be saved to the database. It also includes some quality of life improvements around setting up IMAP and SMTP for our first supported provider, GMail. This PR does not remove the old group email config, that will come in a subsequent PR. This is related to https://meta.discourse.org/t/imap-support-for-group-inboxes/160588 so read that if you would like more backstory.
### UI
Both site settings of `enable_imap` and `enable_smtp` must be true to test this. You must enable SMTP first to enable IMAP.
You can prefill the SMTP settings with GMail configuration. To proceed with saving these settings you must test them, which is handled by the EmailSettingsValidator.
If there is an issue with the configuration or credentials a meaningful error message should be shown.
IMAP settings must also be validated when IMAP is enabled, before saving.
When saving IMAP, we fetch the mailboxes for that account and populate them. This mailbox must be selected and saved for IMAP to work (the feature acts as though it is disabled until the mailbox is selected and saved):
### Database & Backend
This adds several columns to the Groups table. The purpose of this change is to make it much more explicit that SMTP/IMAP is enabled for a group, rather than relying on settings not being null. Also included is an UPDATE query to backfill these columns. These columns are automatically filled when updating the group.
For GMail, we now filter the mailboxes returned. This is so users cannot use a mailbox like Sent or Trash for syncing, which would generally be disastrous.
There is a new group endpoint for testing email settings. This may be useful in the future for other places in our UI, at which point it can be extracted to a more generic endpoint or module to be included.
The default `allow_title` column value is "true" for regular and leader badges. After we disable it in admin side the seed method enabling it again while upgrading. So we shouldn't do it for existing badges.
* FIX: Improve GitHub folder regexp in Onebox
It used to match any GitHub URL that was not matched by the other GitHub
Oneboxes and it did not do a good job at handling those. With this
change, the generic Onebox will handle the remaining URLs.
* FEATURE: Add Onebox for GitHub Actions
* FEATURE: Add Onebox for PR check runs
* FIX: Remove image from GitHub folder Oneboxes
It is a generic, auto-generated image which does not provide any value.
* DEV: Add tests
* FIX: Strip HTML comments from PR body
* UX: alert screen readers when there is an issue saving a post
Adds a "alert" role to various popup-input-tips.
This means screen reader users can now tell why a post refuses to save.
Also ensures like icon in the "try the like button" has screen reader support
Previously auto focus would only work on modals that include buttons or
inputs.
To avoid a situation where information modals such as keyboard shortcuts
do not get focus, simply focus on the close button as a fallback.
Previously we would retry push notifications indefinitely for all errors
except for ExpiredSubscription
Under certain conditions other persistent errors may arise such as a persistent
rate limit.
If we track more than 3 errors in a period of time longer than a day we will
delete the subscription
Also performs a bit of internal cleanup to ensure protected methods really
are private.
Admins can visit an approved queued topic from the review queue by clicking their title. We no longer store the created post and topic ids in the reviewable's payload object. Instead, we set the `topic_id` and `target_id` attributes.
There are two methods which the server uses to verify an invite is being redeemed with a matching email:
1) The email token, supplied via a `?t=` parameter
2) The validity of the email, as provided by the auth provider
Only one of these needs to be true for the invite to be redeemed successfully on the server. The frontend logic was previously only checking (2). This commit updates the frontend logic to match the server.
This commit does not affect the invite redemption logic. It only affects the 'show' endpoint, and the UI.
Previously we had no role set for various topic links, nor did we have any
headers.
This teaches screen readers that topic links in topic lists are to be treated
as H2. We opted for this less radical change cause a change of the element
type would probably result in many broken themes.
Confirmed on NVDA you can very quickly breeze through topic lists now. Minor
edge case is pinned topics which can be a bit annoying due to multiple links.
The previous commits removed reviewables leading to a bad user
experience. This commit updates the status, replaces actions with a
message and greys out the reviewable.
We now bundle Javascript for each theme/plugin separately, and only ship bundles for enabled plugins to the client. Therefore, these disabled_plugins checks are now redundant, and can be removed.
This PR improves the UI of bulk select so that its context is applied to the Dismiss Unread and Dismiss New buttons. Regular users (not just staff) are now able to use topic bulk selection on the /new and /unread routes to perform these dismiss actions more selectively.
For Dismiss Unread, there is a new count in the text of the button and in the modal when one or more topic is selected with the bulk select checkboxes.
For Dismiss New, there is a count in the button text, and we have added functionality to the server side to accept an array of topic ids to dismiss new for, instead of always having to dismiss all new, the same as the bulk dismiss unread functionality. To clean things up, the `DismissTopics` service has been rolled into the `TopicsBulkAction` service.
We now also show the top Dismiss/Dismiss New button based on whether the bottom one is in the viewport, not just based on the topic count.
Re-lands the change initially proposed on #8359 but without a new nginx
location block, so it has less change surface.
Co-authored-by: Jeff Wong <awole20@gmail.com>
Co-authored-by: Jeff Wong <awole20@gmail.com>
Not all screen readers treat articles as navigable roles when moving between landmarks. To help with this, we use a `heading` role on the title, with an arbitrary depth of 2 chosen as to not conflict with the main `<h1/>`. Because ARIA roles are used, this change should be entirely non-visual.
While this introduces minor navigation challenges in posts where headers are included, the vast majority of posts don't, and as screen reader users, we're used to mixed headers in longer-form content.
NVDA does not detect HTML5 articles as regions. This explicitly sets a
region with an aria-label denoting post numbers making it much easier to
know where you are in a topic.
Note role: article which is more semantically correct is not respected by
NVDA d/D shortcut, hence the much more generic "region" role.
When slow mode is enabled it's possible to open the slow mode dialog again to disable it or to update slow mode settings. The problem is that in this case, the button for saving still has the label "Enable" which is confusing.
This changes the text on the button from "Enable" to "Update" when slow mode is already enabled.
Based on feedback from Matt Haughey, we don't need to use so many words when describing a deleted topic or post.
Co-authored-by: Martin Brennan <martin@discourse.org>
This allows plugins to store/modify things in the session (e.g. the destination_url). This change is backwards compatible with existing plugins. If they do not specify a third argument, they will just be passed the first two.
If reload a page after enabling slow mode and open the slow mode dialog again it would show a slow mode interval but wouldn't show Enabled Until value. This PR fixes it.
It used to allow adding email addresses to a group even if invites were
disabled for the site. This does not allow user to input email address
if they cannot invite.
The second thing this commit improves is the message that is displayed
to the user when they hit the invite rate limit.
Over the years we accrued many spelling mistakes in the code base.
This PR attempts to fix spelling mistakes and typos in all areas of the code that are extremely safe to change
- comments
- test descriptions
- other low risk areas
This commit fixes an issue where controls scroll in lightboxes with large images (after zooming in)
Before:
05024730b3.mp4
Notice how controls like the close button, the next and previous button, and the image metadata also scroll? This is an undesired behavior.
After:
8047bab735.mp4
This is the desired behavior; only the image should scroll.
The changes in this PR apply to both desktop and mobile.
The problem was we were setting the properties then immediately calling
`refreshRoute` which was being executed before the properties were
settled via the runloop.
Under certain conditions admins would miss messages when posting action in
topics where they have permission.
This also fixes an error where we would sometimes explode when publishing to
an empty group.
> Backtracking re-render refers to a scenario where, in the middle of the rendering process, you have modified something that has already been rendered.
See more details from the Ember team here https://github.com/emberjs/ember.js/issues/13948.
We call _updateInput from init. _updateInput triggers onChangeInput which mutates a date that was given to future-date-input just a moment ago and a rendering cycle wasn't finished yet.
The crash:
```
Uncaught TypeError: Ember.keys is not a function
```
Repro:
- visit home page
- click new topic
- navigate to your messages by clicking your avatar (top right), then enveloppe icon, and finally the bottom chevron
- click New Message
- click cancel in the composer, it should crash
Watched words are always regular expressions, despite watched_words_
_regular_expressions being enabled or not. Internally, wildcard
characters are replaced with a regular expression that matches any non
whitespace character.
This is two fixes:
1. Ember CLI's proxy did not support 3xx redirects so a redirect was
failing.
2. We were not passing query parameters to the `bootstrap.json` endpoint
to correctly handle previewing themes (and other occasional options.)
If you finished reviewing the initially loaded items, and there're more in the queue, load them.
Also, when fast-tracking the pending items updates, use the reviewable_count returned by the perform result. Calling "result.reviewable_count" returns undefines.
This patch remembers the last id for the `file-change` event and uses it
to initialize the client side watcher. This should help fix the issue
where styles are not reloaded client side if the browser refreshed.
* FIX: Hide tag watched words if tagging is disabled
These 'autotag' words were shown even if tagging was disabled.
* FIX: Make autotag watched words case insensitive
This commit also fixes the bug when no tag was applied if no other tag
was already present.
Email change requests are never deleted no matter if they completed
successfully or not. The abandoned requests have the disadvantage of
showing up as unconfirmed emails in user's preferences page.
This is a recent regression introduced by https://github.com/discourse/discourse/pull/12937 which makes it so that when looking at a user profile that is not your own, specifically the category and tag notification settings, you would see your own settings instead of the target user. This is only a problem for admins because regular users cannot see these details for other users.
The issue was that we were using `scope` in the serializer, which refers to the current user, rather than using a scope for the target user via `Guardian.new(user)`.
However, on further inspection the `notification_levels_for` method for `TagUser` and `CategoryUser` did not actually need to be accepting an instance of Guardian, all that it was using it for was to check guardian.anonymous? which is just a fancy way of saying user.blank?. Changed this method to just accept a user instead and send the user in from the serializer.
We have a few places in the code where we need to validate various email related settings, and will have another soon with the improved group email settings UI. This PR introduces a class which can validate POP3, IMAP, and SMTP credentials and also provide a friendly error message for issues if they must be presented to an end user.
This PR does not change any existing code to use the new service. I have added a TODO to change POP3 validation and the email test rake task to use the new validator post-release.
Normally we'd use `ember-auto-import` for this, but it's not run on
our admin tree due to the quirky way we load it conditionally.
Instead we'll append it at the bottom like our Rails app does.
* FIX: Ensure the same email cannot be invited twice
When creating a new invite with a duplicated email, the old invite will
be updated and returned. When updating an invite with a duplicated email
address, an error will be returned.
* FIX: not Ember helper does not exist
* FIX: Sync can_invite_to_forum? and can_invite_to?
The two methods should perform the same basic set of checks, such as
check must_approve_users site setting.
Ideally, one of the methods would call the other one or be merged and
that will happen in the future.
* FIX: Show invite to group if user is group owner
Some themes/components depend on plugins, and it would be impossible to write tests for those themes without installing/loading the plugins they depend on.
Identical callbacks can pile up during tests and cause all sort of weird problems that are difficult to debug. This commit clears registered callbacks after each test.
Recalculating a ReviewableFlaggedPost's score after rejecting or ignoring it sets the score as 0, which means that we can't find them after reviewing. They don't surpass the minimum priority threshold and are hidden.
Additionally, we only want to use agreed flags when calculating the different priority thresholds.
* FIX: ember-cli proxy subfolder fix
* REFACTOR: put rootURL setup in environment, update baseURL logic for subfolder
Correctly have ember understand and parse relative_url_root and use it in
the dev server
Some emails coming in via the mail receiver can still end up
with bad encoding when trying to enqueue the job. This catches
the last encoding issue and forces iso-8559-1 and encodes to
UTF-8 to circumvent the issue.
This was needed to fix a bookmark back button issue but it
broke category topic links, causing a full reload. Now it appears
something has changed in core and this is no longer necessary for
the bookmark back button to work, so I am removing it again.
Currently, when the target is not available we're returning the error message "`You are not permitted to view the requested resource`" which is not clear.
* DEV: add a method of skipping publishing stylesheets afer color scheme save
allows a method to publish all stylesheets if we make changes to many
stylesheets at once
* use after_save_commit for stylesheet change callbacks
This may be more reliable for picking up new stylesheet changes via messagebus
as after_save does not guarantee the updates exists in the DB yet.
* add skip_publish option for create_from_base
* FIX: Cache missing inline oneboxes
Some inline oneboxes were not cached when the server did not return an
answer for an URL and the queried URL and the absolute URL were
different.
For example, if user typed www.example.com, the client asked the server
for http://www.example.com and if the server returned an empty response,
then the client would keep requesting an inline onebox everytime the
composer changed.
In other words, the key used for reading (the absolute URL) and the one
used for writing (the URL as typed by the user) were not the same when
the server returned an empty response.
* DEV: Check cache before making request
There is another cache check in PrettyText, but that is not enough if
multiple requests are pending. This problem was made obvious in tests,
but can happen for users with slow connections.
* FEATURE: Allow sending a message with invite
It used to be a staff-only feature and this commit makes it available
to everyone who can invite.
* FIX: Inviting to topic uses another email template
This used to be the case, but the extra parameter was lost when we
switched to the new modal.
We have found when receiving and posting inbound emails to the handle_mail route, it is better to POST the payload as a base64 encoded string to avoid strange encoding issues. This introduces a new param of `email_encoded` and maintains the legacy param of email, showing a deprecation warning. Eventually the old param of `email` will be dropped and the new one `email_encoded` will be the only way to handle_mail.
The user may have changed their category or tag tracking settings since a topic was tracked/watched based on those settings in the past. In that case we need to alter the reason message we show them otherwise it is very confusing for the end user to be told they are tracking a topic because of a category, when they are no longer tracking that category.
For example: "You will see a count of new replies because you are tracking this category." becomes: "You will see a count of new replies because you were tracking this category in the past."
To do this, it was necessary to add tag and category tracking info to current user serializer. I improved the serializer code so it only does 3 SQL queries instead of 9 to get the tracking information for tags and categories for the current user.
It's been awhile since we have supported IE11 so it should be safe to remove
IntersectionObserver now.
From a TODO task in this repo:
> drop when we eventually drop IE11
Announcement of when we removed IE11 support:
https://meta.discourse.org/t/137984/40?u=blake
See: https://meta.discourse.org/t/navigating-back-to-bookmarks/188912/4
Instead of taking the user back to the bookmark list after selecting
a topic and navigating back, the user was navigated back to the page
before that. This is because the topic-link component was missing
the data-auto-route attribute which tells the intercept-click library
not to use DiscourseURL.routeTo to handle the transition (so it is just
handled internally by Ember)
When editing the files for a theme in the admin dashboard, typing "cmd+s" (a common key-binding to save in most text editors) used to engage the browser's default "save page" dialogue.
This commit adds a key-binding to the ace editor that saves the file.
Now, the "cmd+s" (and "ctrl+s" for windows) key-binding does the same action as the save button.
This commit will add CSS classes like `unlisted`, `pinned`, and `unpinned` on the body tag.
* DEV: we no longer using the `categoryClass` & `tagClasses` methods.
* Update app/assets/javascripts/discourse/app/components/add-topic-status-classes.js
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
