It allows saving local date to calendar.
Modal is giving option to pick between ics and google. User choice can be remembered as a default for the next actions.
* FEATURE: Return subcategories on categories endpoint
When using the API subcategories will now be returned nested inside of
each category response under the `subcategory_list` param. We already
return all the subcategory ids under the `subcategory_ids` param, but
you then would have to make multiple separate API calls to fetch each of
those subcategories. This way you can get **ALL** of the categories
along with their subcategories in a single API response.
The UI will not be affected by this change because you need to pass in
the `include_subcategories=true` param in order for subcategories to be
returned.
In a follow up PR I'll add the API scoping for fetching categories so
that a readonly API key can be used for the `/categories.json` endpoint. This
endpoint should be used instead of the `/site.json` endpoint for
fetching a sites categories and subcategories.
* Update PR based on feedback
- Have spec check for specific subcategory
- Move comparison check out of loop
- Only populate subcategory list if option present
- Remove empty array initialization
- Update api spec to allow null response
* More PR updates based on feedback
- Use a category serializer for the subcategory_list
- Don't include the subcategory_list param if empty
- For the spec check for the subcategory by id
- Fix spec to account for param not present when empty
The host's category was successfully updated on the database, but the category property was not properly set when rendering the component for the first time.
* FIX: Stop tracking incoming message after navigating away take 2.
Previous fix in d82e5cd37c resulted in
counts being flappy as we cleared the active inbox between routes.
Co-authored-by: Osama Sayegh <asooomaasoooma90@gmail.com>
We relied on backticks to identify and replace site setting names with links. Unfortunately, some translations don't follow this convention, breaking this feature.
Additionally, this lets us linkify `category settings` and `watched words` without using HTML in the translations.
You may notice that I split the texts we want to linkify into two groups. I did this on purpose to emphasize those that should be translated (regular_links) from those who don't (site_settings_link). If you can think of a better solution, I'm open to suggestions.
* DEV: Remove HTML setting type and sanitization logic.
We concluded that we don't want settings to contain HTML, so I'm removing the setting type and sanitization logic. Additionally, we no longer allow the global-notice text to contain HTML.
I searched for usages of this setting type in the `all-the-plugins` repo and found none, so I haven't added a migration for existing settings.
* Mark Global notices containing links as HTML Safe.
After adding an empty state banner to the user bookmarks page, we have found the bug. Steps to reproduce:
- Go to the user bookmarks page
- Search for something that doesn’t exist in bookmarks
- Click again Bookmarked on the sidebar or View All Bookmarks on the user menu again
FinalDestination now supports the `follow_canonical` option, which will perform an initial GET request, parse the canonical link if present, and perform a HEAD request to it.
We use this mode during embeds to avoid treating URLs with different query parameters as different topics.
Previously we would store every FakeRequest object for all tests, resulting in many hundreds/thousands of objects in the `handledRequests` array.
This commit ensures all pretender state is reset between tests.
It was possible to see notifications of other users using routes:
- notifications/responses
- notifications/likes-received
- notifications/mentions
- notifications/edits
We weren't showing anything private (like notifications about private messages), only things that're publicly available in other places. But anyway, it feels strange that it's possible to look at notifications of someone else. Additionally, there is a risk that we can unintentionally leak something on these pages in the future.
This commit restricts these routes.
- There's no need to pass `filter` to `user-notifications-large`. The component doesn't use it.
- Rename css class to avoid confusion (this div has nothing to-do with the Select Kit)
- Remove duplicated declarations in test fixtures
This is `console.log`'d to the browser console. run-qunit will print this to stdout. testem will not, so a custom reporter is implemented to print this message.
The `--enable-precise-memory-info` is added so that chrome provides high-resolution memory information. This API is not supported by firefox. The logic will degrade gracefully.
Note this commit is also adding support for teardown in pre-initializers just like we have for initializers.
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Co-authored-by: David Taylor <david@taylorhq.com>
We were using multiple methods to check which environment we're running in. This commit switches us to use the isLegacyEmber helper consistently. This should be a no-op, but makes the code much easier to read
Under Ember CLI, we create a new application instance for each test. We were not correctly destroying it after the test, causing many references to be maintaned (e.g. at the end of a test run, `Ember.Namespace.NAMESPACES` would have an entry for each application instance).
Calling `destroy` on the application instance tidies up these references, and is one step towards fixing our test memory leak problem. Unfortunately there still seem to be other references being held to the application, so this commit is not a total fix.
The all inboxes was introduced in
016efeadf6 but we decided to roll it back
for performance reasons. The main performance challenge here is that PG
has to basically loop through all the PMs that a user is allowed to view
before being able to order by `Topic#bumped_at`. The all inboxes was not
planned as part of the new/unread filter so we've decided not to tackle
the performance issue for the upcoming release.
Follow-up to 016efeadf6
* PERF: Improve database query perf when loading topics for a category.
Instead of left joining the `topics` table against `categories` by filtering with `categories.id`,
we can improve the query plan by filtering against `topics.category_id`
first before joining which helps to reduce the number of rows in the
topics table that has to be joined against the other tables and also
make better use of our existing index.
The following is a before and after of the query plan for a category
with many subcategories.
Before:
```
QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------
-----------------------------------------------------------------------------------
Limit (cost=1.28..747.09 rows=30 width=12) (actual time=85.502..2453.727 rows=30 loops=1)
-> Nested Loop Left Join (cost=1.28..566518.36 rows=22788 width=12) (actual time=85.501..2453.722 rows=30 loops=1)
Join Filter: (category_users.category_id = topics.category_id)
Filter: ((topics.category_id = 11) OR (COALESCE(category_users.notification_level, 1) <> 0) OR (tu.notification_level > 1))
-> Nested Loop Left Join (cost=1.00..566001.58 rows=22866 width=20) (actual time=85.494..2453.702 rows=30 loops=1)
Filter: ((COALESCE(tu.notification_level, 1) > 0) AND ((topics.category_id <> 11) OR (topics.pinned_at IS NULL) OR ((t
opics.pinned_at <= tu.cleared_pinned_at) AND (tu.cleared_pinned_at IS NOT NULL))))
Rows Removed by Filter: 1
-> Nested Loop (cost=0.57..528561.75 rows=68606 width=24) (actual time=85.472..2453.562 rows=31 loops=1)
Join Filter: ((topics.category_id = categories.id) AND ((categories.topic_id <> topics.id) OR (categories.id = 1
1)))
Rows Removed by Join Filter: 13938306
-> Index Scan using index_topics_on_bumped_at on topics (cost=0.42..100480.05 rows=715549 width=24) (actual ti
me=0.010..633.015 rows=464623 loops=1)
Filter: ((deleted_at IS NULL) AND ((archetype)::text <> 'private_message'::text))
Rows Removed by Filter: 105321
-> Materialize (cost=0.14..36.04 rows=30 width=8) (actual time=0.000..0.002 rows=30 loops=464623)
-> Index Scan using categories_pkey on categories (cost=0.14..35.89 rows=30 width=8) (actual time=0.006.
.0.040 rows=30 loops=1)
Index Cond: (id = ANY ('{11,53,57,55,54,56,112,94,107,115,116,117,97,95,102,103,101,105,99,114,106,1
13,104,98,100,96,108,109,110,111}'::integer[]))
-> Index Scan using index_topic_users_on_topic_id_and_user_id on topic_users tu (cost=0.43..0.53 rows=1 width=16) (a
ctual time=0.004..0.004 rows=0 loops=31)
Index Cond: ((topic_id = topics.id) AND (user_id = 1103877))
-> Materialize (cost=0.28..2.30 rows=1 width=8) (actual time=0.000..0.000 rows=0 loops=30)
-> Index Scan using index_category_users_on_user_id_and_last_seen_at on category_users (cost=0.28..2.29 rows=1 width
=8) (actual time=0.004..0.004 rows=0 loops=1)
Index Cond: (user_id = 1103877)
Planning Time: 1.359 ms
Execution Time: 2453.765 ms
(23 rows)
```
After:
```
QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Limit (cost=1.28..438.55 rows=30 width=12) (actual time=38.297..657.215 rows=30 loops=1)
-> Nested Loop Left Join (cost=1.28..195944.68 rows=13443 width=12) (actual time=38.296..657.211 rows=30 loops=1)
Filter: ((categories.topic_id <> topics.id) OR (topics.category_id = 11))
Rows Removed by Filter: 29
-> Nested Loop Left Join (cost=1.13..193462.59 rows=13443 width=16) (actual time=38.289..657.092 rows=59 loops=1)
Join Filter: (category_users.category_id = topics.category_id)
Filter: ((topics.category_id = 11) OR (COALESCE(category_users.notification_level, 1) <> 0) OR (tu.notification_level > 1))
-> Nested Loop Left Join (cost=0.85..193156.79 rows=13489 width=20) (actual time=38.282..657.059 rows=59 loops=1)
Filter: ((COALESCE(tu.notification_level, 1) > 0) AND ((topics.category_id <> 11) OR (topics.pinned_at IS NULL) OR ((topics.pinned_at <= tu.cleared_pinned_at) AND (tu.cleared_pinned_at IS NOT NULL))))
Rows Removed by Filter: 1
-> Index Scan using index_topics_on_bumped_at on topics (cost=0.42..134521.06 rows=40470 width=24) (actual time=38.267..656.850 rows=60 loops=1)
Filter: ((deleted_at IS NULL) AND ((archetype)::text <> 'private_message'::text) AND (category_id = ANY ('{11,53,57,55,54,56,112,94,107,115,116,117,97,95,102,103,101,105,99,114,106,113,104,98,100,96,108,109,110,111}'::integer[])))
Rows Removed by Filter: 569895
-> Index Scan using index_topic_users_on_topic_id_and_user_id on topic_users tu (cost=0.43..1.43 rows=1 width=16) (actual time=0.003..0.003 rows=0 loops=60)
Index Cond: ((topic_id = topics.id) AND (user_id = 1103877))
-> Materialize (cost=0.28..2.30 rows=1 width=8) (actual time=0.000..0.000 rows=0 loops=59)
-> Index Scan using index_category_users_on_user_id_and_last_seen_at on category_users (cost=0.28..2.29 rows=1 width=8) (actual time=0.004..0.004 rows=0 loops=1)
Index Cond: (user_id = 1103877)
-> Index Scan using categories_pkey on categories (cost=0.14..0.17 rows=1 width=8) (actual time=0.001..0.001 rows=1 loops=59)
Index Cond: (id = topics.category_id)
Planning Time: 1.633 ms
Execution Time: 657.255 ms
(22 rows)
```
* PERF: Optimize index on topics bumped_at.
Replace `index_topics_on_bumped_at` index with a partial index on `Topic#bumped_at` filtered by archetype since there is already another index that covers private topics.
As sharing has some hover behavior, it was looking slightly clunky with fast edit changing position. Putting sharing at the last position will reduce this effect.
When the loading spinner is removed (e.g. via the loading-slider component), the subcategory list view will persist, even when no longer required. This is because we were conditionally rendering the list into the `header-list-container` outlet. When the condition was false, we were doing nothing. Instead, we should use `disconectOutlet` to make sure the content is removed from the DOM.
Firefox does not return a PerformanceMeasure object when using
performance.mark and performance.measure, even though MDN says it
should https://developer.mozilla.org/en-US/docs/Web/API/Performance/measure#return_value
So for now, we disable the upload instrumentation with a test
to see if a PerformanceMeasure (or anything really) is returned.
When creating a reply after already navigating out of the
topic (e.g. open the reply composer, go to a different topic,
then create the post), the _removeDeleteOnOwnerReplyBookmarks
function was erroring because it relied on the topic model
being present.
We can skip this function altogether if the topic model is _not_
present, because the PostCreator already takes care of deleting
bookmarks with the on_owner_reply auto_delete_preference. The
_removeDeleteOnOwnerReplyBookmarks function just cleans up the
in-memory post stream and topic model.
In the user bookmark list, when we show the excerpt of the bookmark
(which is usually just the bookmarked post excerpt), we want to show
the first unread post's excerpt instead for for_topic bookmarks. This
is because when the user clicks on that bookmark link, they are taken
to the first unread post in the topic, not the OP, as per:
27699648ef
This commit allows for measuring the time taken for
individual uploads via the new uppy interfaces, only
if the enable_upload_debug_mode site setting is enabled.
Also in this PR, for upload errors with a specific message
locally, we return the real message to show in the modal
instead of the upload.failed message so the developer
does not have to dig around in logs.
The file size error messages for max_image_size_kb and
max_attachment_size_kb are shown to the user in the KB
format, regardless of how large the limit is. Since we
are going to support uploading much larger files soon,
this KB-based limit soon becomes unfriendly to the end
user.
For example, if the max attachment size is set to 512000
KB, this is what the user sees:
> Sorry, the file you are trying to upload is too big (maximum
size is 512000KB)
This makes the user do math. In almost all file explorers that
a regular user would be familiar width, the file size is shown
in a format based on the maximum increment (e.g. KB, MB, GB).
This commit changes the behaviour to output a humanized file size
instead of the raw KB. For the above example, it would now say:
> Sorry, the file you are trying to upload is too big (maximum
size is 512 MB)
This humanization also handles decimals, e.g. 1536KB = 1.5 MB
This commit also hides a number of options which are not used during Discourse development.
Change have been tested on both the legacy `/qunit` route, and the Ember CLI `/tests` route.
This adds support for `qunit_skip_core`, `qunit_skip_plugins` and `qunit_single_plugin` parameters on the Ember CLI `/tests` route using the `addModuleExcludeMatcher` API. Legacy support is maintained for the `/qunit` route.
".search-menu" matches the parent element of the element that was
previously selected. This is a better choice because it offers some
flexibility over the DOM structure without breaking the keyboard
shortcuts.
Instead of going to the OP of the topic for topic-level bookmarks
(which are bookmarks where for_topic is true) when clicking on the
bookmark in the quick access menu or on the user bookmark list,
this commit takes the user to the last unread post in
the topic instead. This should be generally more useful than landing
on the unchanging OP.
To make this work nicely, I needed to add the last_read_post_number to
the BookmarkQuery based on the TopicUser association. It should not add
too much extra weight to the query, because it is limited to the user
that we are fetching bookmarks for.
Also fixed an issue where the bookmark serializer highest_post_number was
not taking into account whether the user was staff, which is when we
should use highest_staff_post_number instead.
* DEV: use active record `save!` instead of mini sql.
The "save" method will trigger the before_save callback "match_primary_group_changes" for User model. Else `flair_group_id` won't be removed from the user.
* check whether the method `match_primary_group_changes` called or not.
Allows creating a bookmark with the `for_topic` flag introduced in d1d2298a4c set to true. This happens when clicking on the Bookmark button in the topic footer when no other posts are bookmarked. In a later PR, when clicking on these topic-level bookmarks the user will be taken to the last unread post in the topic, not the OP. Only the OP can have a topic level bookmark, and users can also make a post-level bookmark on the OP of the topic.
I had to do some pretty heavy refactors because most of the bookmark code in the JS topics controller was centred around instances of Post JS models, but the topic level bookmark is not centred around a post. Some refactors were just for readability as well.
Also removes some missed reminderType code from the purge in 41e19adb0d
We want to be able to skip plugins from doing any work under
certain conditions, and to be able raise their own errors if
a file being uploaded is completely incompatible with the concept
of the plugin if it is enabled. For example, the UppyChecksum plugin
is happy to skip hashing large files, but the UppyUploadEncrypt
plugin from discourse-encrypt relies on the file being encrypted
to do anything with the upload, so it is considered a blocking
error if the user uploads a file that is too large.
This improves the base functions available in uppy-plugin-base and
extendable-uploader to handle this, as well as introducing a
HUGE_FILE_THRESHOLD_BYTES variable which represents 100MB in bytes,
matching the ExternalUploadManager::DOWNLOAD_LIMIT on the
server side.
discourse-encrypt to take advantage of this new functionality will
follow in discourse/discourse-encrypt#141
Also promote the `create_notification_alert` and `push_notification`
methods from instance methods to class methods so that plugins can call
them. This is temporary until we add a more comprehensive API for
extending `PostAlerter`.
We want to be able to skip plugins from doing any work under
certain conditions, and to be able raise their own errors if
a file being uploaded is completely incompatible with the concept
of the plugin if it is enabled. For example, the UppyChecksum plugin
is happy to skip hashing large files, but the UppyUploadEncrypt
plugin from discourse-encrypt relies on the file being encrypted
to do anything with the upload, so it is considered a blocking
error if the user uploads a file that is too large.
This improves the base functions available in uppy-plugin-base and
extendable-uploader to handle this, as well as introducing a
HUGE_FILE_THRESHOLD_BYTES variable which represents 100MB in bytes,
matching the ExternalUploadManager::DOWNLOAD_LIMIT on the
server side.
discourse-encrypt to take advantage of this new functionality will
follow in https://github.com/discourse/discourse-encrypt/pull/141
After deleting a category, we should soft-delete the category definition topic instead of hard deleting it. Else it causes issues while doing the user merge action if the source user has an orphan post that belongs to the deleted topic.
- do not reduce opacity of disabled buttons if they are loading
- replace ‘|’ by single quotes not double quotes
- always start from index 0
- reduces amount of work by checking row's length
- apply quotefix to fallback
- do not add 1 to caretposition if index is 0
The algorithm will now do the following:
- split selection to retain only first line
- removes possible "* "
- check for first inclusion
- fallback to first row if nothing found
We don't actually use the reminder_type for bookmarks anywhere;
we are just storing it. It has no bearing on the UI. It used
to be relevant with the at_desktop bookmark reminders (see
fa572d3a7a)
This commit marks the column as readonly, ignores it, and removes
the index, and it will be dropped in a later PR. Some plugins
are relying on reminder_type partially so some stubs have been
left in place to avoid errors.
This partially reverts commit ddb458343d.
Seeing performance degrade on larger sites so back to drawing board on
this one. Instead of the DISTINCT LEFT JOIN, we switch back to
IN(subquery).
First reported in https://meta.discourse.org/t/-/202482/19
There are two optimizations being applied here:
1. Fetch a user's group ids in a seperate query instead of including it
as a sub-query. When I tried a subquery, the query plan becomes very
inefficient.
1. Join against the `topic_allowed_users` and `topic_allowed_groups`
table instead of doing an IN against a subquery where we UNION the
`topic_id`s from the two tables. From my profiling, this enables PG to
do a backwards index scan on the `index_topics_on_timestamps_private`
index.
This commit fixes a bug where listing all messages was incorrectly
excluding topics if a topic has been archived by a group even if the
user did not belong to the group.
This commit also fixes another bug where dismissing private messages
selectively was subjected to the default limit of 30.
This new column will be used to indicate that a bookmark
is at the topic level. The first post of a topic can be
bookmarked twice after this change -- with for_topic set
to true and with for_topic set to false.
A later PR will use this column for logic to bookmark the
topic, and then topic-level bookmark links will take you
to the last unread post in the topic.
See also 22208836c5
We don't need no stinkin' denormalization! This commit ignores
the topic_id column on bookmarks, to be deleted at a later date.
We don't really need this column and it's better to rely on the
post.topic_id as the canonical topic_id for bookmarks, then we
don't need to remember to update both columns if the bookmarked
post moves to another topic.
Discourse is sending regularly message to admins when potential problems are persisted. Most of the time they have exactly the same content. In that case, when there are no replies, the old one should be trashed before a new one is created.
This commit sets `tap_failed_tests_only` to `true` in our testem config, so now only the failing tests will show in our GitHub CI Ember test runs, which saves developers from having to hunt through all of the passing tests using GitHub's janky console output scrollback.
There was a check for closed code blocks (which had both opening and
closing markups), but it did not work for the case when the text ends
in an open code block.
Administrators can use second factor to confirm granting admin access
without using email. The old method of confirmation via email is still
used as a fallback when second factor is unavailable.
The previous excerpt was a simple truncated raw message. Starting with
this commit, the raw content of the draft is cooked and an excerpt is
extracted from it. The logic for extracting the excerpt mimics the the
`ExcerptParser` class, but does not implement all functionality, being
a much simpler implementation.
The two draft controllers have been merged into one and the /draft.json
route has been changed to /drafts.json to be consistent with the other
route names.
This is necessary to allow for large file uploads via
the direct S3 upload mechanism, as we convert the external
file to an Upload record via ExternalUploadManager once
it is complete.
This will allow for files larger than 2,147,483,647 bytes (2.14GB)
to be referenced in the uploads table.
This is a table locking migration, but since it is not as highly
trafficked as posts, topics, or users, the disruption should be minimal.
This is my second try at this. The first b246a63a59 raised an issue
with the event delegation not working because the topic id changed.
This adds support for delegating events to dynamic keys by passing a
function where a static key would normally be needed. This means that
each timeline will have its own unique state key and events will only
delegate to the proper topic.
Translations are often multi-line. Using a regular `<input>` doesn't allow newlines, so if you try to edit a multiline theme translation, all the line breaks will be removed.
This commit updates the theme translations UI to use `<textarea>`, just like the core translation editing UI.
allowUpload can be false for the composer if there are no
allowed file extensions. This causes the _bindMobileUploadButton
code to fail because the button does not get rendered in the
template if !allowUpload. This commit changes composer-editor
to only bind upload functionality if allowUpload.
We've observed an error where the back button is displayed improperly in
the topic timeline. It's unfortunately been hard to reproduce but we
suspect it's related to leftover state when re-rendering.
This fix optimistically tries to fix the error by introducing the
topic's id to the unique key the widgets use for state. We can deploy
this and keep an eye out for the bug in the future.
This fixes an error when trying to upload a profile
background image for the user card when the
enable_direct_s3_uploads setting was true:
> Failed to execute 'send' on 'XMLHttpRequest': The object's state must be OPENED.
This was fixed in the upstream commit by the uppy devs:
5937bf2127
When a user archives a personal message, they are redirected back to the
inbox and will refresh the list of the topics for the given filter.
Publishing an event to the user results in an incorrect incoming message
because the list of topics has already been refreshed.
This does mean that if a user has two tabs opened, the non-active tab
will not receive the incoming message but at this point we do not think
the technical trade-offs are worth it to support this feature. We
basically have to somehow exclude a client from an incoming message
which is not easy to do.
Follow-up to fc1fd1b416
This abstracts interaction with uppy for uppy plugin classes
into base classes for Preprocessor plugins, so anyone
making these uppy plugins doesn't have to think as much about uppy
underneath the hood. This also makes the logging and validation
nicer, and provides a more consistent way to emit progress and
completion events.
In a future commit, we will introduce another base class for
`UploadUploaderPlugin` which will be used to be able to hijack
the upload process to go to a different provider (e.g. for discourse-video)
Short URLs were resolved before diffHTML was loaded and content was
swapped by it, which meant that no URLs were found and the URLs remained
unsolved. This caused image elements to be blank.
* DEV: Updated diffHTML to 1.0.0-beta.20
Watched words of type 'replace' or 'link' replaced the text inside
mentions or hashtags too, which broke these. These types of watched
words must skip any match that has an @ or # before it.
At this point in time, we do not think supporting unread and new when an
admin is looking at another user's messages is worth supporting.
Follow-up to fc1fd1b416
This was problematic if something like SCSS file throws an error as the
app would tell Ember CLI to bootstrap as if everything is fine and not
display the error.
The fix is to only hijack the rendering at the end of the template
instead of the beginning.
This commit updates the RSS post content to use email formatting. Many
plugins are using the `reduce_cooked` method to format content that is
not displayed outside of Discourse application. Using email formatting
also strips the secure media and various other things that is only meant
for Discourse client side application.
The modification date should always be a meta tag to make this less confusing. Especially for imported posts.
That's more in line with how the rest of Discourse presents post dates.
There are a few fixes at play here:
1) We were still not initializing objects to the correct types.
2) If a debounce timed out, it was returning a string instead of an
array which was not appropriately handled.
3) In testing mode we never cancel the search promise for stability.
In order to include the new/unread count in the browse more message
under suggested topics, a couple of technical changes have to be made.
1. `PrivateMessageTopicTrackingState` is now auto-injected which is
similar to how it is done for `TopicTrackingState`. This is done so
we don't have to attempt to pass the `PrivateMessageTopicTrackingState`
object multiple levels down into the suggested-topics component. While
the object is auto-injected, we only fetch the initial state and start
tracking when the relevant private messages routes has been hit and only
when a private message's suggested topics is loaded. This is
done as we do not want to add the extra overhead of fetching the inital
state to all page loads but instead wait till the private messages
routes are hit.
2. Previously, we would stop tracking once the `user-private-messages`
route has been deactivated. However, that is not ideal since
navigating out of the route and back means we send an API call to the
server each time. Since `PrivateMessageTopicTrackingState` is kept in
sync cheaply via messageBus, we can just continue to track the state
even if the user has navigated away from the relevant stages.
The smoke test has been failing with the error:
```
TypeError: Cannot read properties of undefined (reading 'Core')
```
Since de20c46077
and 9873a942e3 this error has been occurring,
possibly now because Uppy is required by a plugin. Adding uppy.js into
the require list for theme_qunit_vendor.js fixes the issue.
This new interface will be used explicitly to add upload
preprocessors in the form of uppy plugins. These will be
run for each upload in the composer (dependent on the logic
of the plugin itself), before the UppyChecksum plugin is
finally run.
Since discourse-encrypt uses the existing addComposerUploadHandler
API for essentially preprocessing an upload and not uploading it
to a different place, it will be the first plugin to use this interface,
along with the register-media-optimization-upload-processor initializer
in core.
Related https://github.com/discourse/discourse-encrypt/pull/131.
Improves the create account modal for screen readers by doing the following:
* Making the `modal-alert` section into an `aria-role="alert"` region and making it show and hide using height instead of display:none so screen readers pick it up. Made a change so the field-related error messages are always shown beneath the field.
* Add `aria-invalid` and `aria-describedby` attributes to each field in the modal, so the screen reader will read out the error hint on error. This necessitated an Ember component extension to allow both the `aria-*` attributes to be bound and to render on `{{input}}`.
* Moved the social login buttons to the right in the HTML structure so they are not read out first.
* Added `aria-label` attributes to the login buttons so they can have different content for screen readers.
* In some cases for modals, the title that should be used for the `aria-labelledby` attribute is within the modal content and not the discourse-modal-title title. This introduces a new titleAriaElementId property to the d-modal component that is then used by the create-account modal to read out the title
------
This is the same as e0d2de73d8 but
fixes the Ember-input-component-extension to use the public
Ember components TextField and TextArea instead of the private
TextSupport so the extension works in both normal Ember and
Ember CLI.
Improves the create account modal for screen readers by doing the following:
* Making the `modal-alert` section into an `aria-role="alert"` region and making it show and hide using height instead of display:none so screen readers pick it up. Made a change so the field-related error messages are always shown beneath the field.
* Add `aria-invalid` and `aria-describedby` attributes to each field in the modal, so the screen reader will read out the error hint on error. This necessitated an Ember component extension to allow both the `aria-*` attributes to be bound and to render on `{{input}}`.
* Moved the social login buttons to the right in the HTML structure so they are not read out first.
* Added `aria-label` attributes to the login buttons so they can have different content for screen readers.
* In some cases for modals, the title that should be used for the `aria-labelledby` attribute is within the modal content and not the discourse-modal-title title. This introduces a new titleAriaElementId property to the d-modal component that is then used by the create-account modal to read out the
This adds a new property, `pluginId` which you can pass to `modifyClass`
which prevent the class from being modified over and over again.
This also includes a fix for polls which was leaking state between tests
which this new functionality exposed.
When using ComposerUpload and/or ComposerUploadUppy, we were
always calling bindMobileUploadButton. However with more composer-like
interfaces being developed, we need this to be optional, as not
everywhere will have a separate mobile upload button to bind to.
Also makes it so the composer extending the ComposerUpload mixins is
responsible for explicitly unbinding the mobile upload button if
it needs to.
DEV: Allow passing cook_method to TopicEmbed.import to override default
This will be used in the rss-polling plugin when we want to have
oneboxes on feed content, like youtube for example.
Previously, a group's `default_notification_level` change will only affect the users added after it.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
* DEV: Use named parameters for dir-span helper
Follow up to: e50a5c0c73
In order to improve code clarity this change introduces named parameters
for the dir-span helper. This is specifically for the new `htmlSafe`
parameter which you can use instead of just passing in a boolean if the
strings you are passing in have already been escaped.
Before: `{{dir-span category.description false}}`
After: `{{dir-span category.description htmlSafe=true}}`
* Set default value for params arg
PresenceChannel aims to be a generic system for allow the server, and end-users, to track the number and identity of users performing a specific task on the site. For example, it might be used to track who is currently 'replying' to a specific topic, editing a specific wiki post, etc.
A few key pieces of information about the system:
- PresenceChannels are identified by a name of the format `/prefix/blah`, where `prefix` has been configured by some core/plugin implementation, and `blah` can be any string the implementation wants to use.
- Presence is a boolean thing - each user is either present, or not present. If a user has multiple clients 'present' in a channel, they will be deduplicated so that the user is only counted once
- Developers can configure the existence and configuration of channels 'just in time' using a callback. The result of this is cached for 2 minutes.
- Configuration of a channel can specify permissions in a similar way to MessageBus (public boolean, a list of allowed_user_ids, and a list of allowed_group_ids). A channel can also be placed in 'count_only' mode, where the identity of present users is not revealed to end-users.
- The backend implementation uses redis lua scripts, and is designed to scale well. In the future, hard limits may be introduced on the maximum number of users that can be present in a channel.
- Clients can enter/leave at will. If a client has not marked itself 'present' in the last 60 seconds, they will automatically 'leave' the channel. The JS implementation takes care of this regular check-in.
- On the client-side, PresenceChannel instances can be fetched from the `presence` ember service. Each PresenceChannel can be used entered/left/subscribed/unsubscribed, and the service will automatically deduplicate information before interacting with the server.
- When a client joins a PresenceChannel, the JS implementation will automatically make a GET request for the current channel state. To avoid this, the channel state can be serialized into one of your existing endpoints, and then passed to the `subscribe` method on the channel.
- The PresenceChannel JS object is an ember object. The `users` and `count` property can be used directly in ember templates, and in computed properties.
- It is important to make sure that you `unsubscribe()` and `leave()` any PresenceChannel objects after use
An example implementation may look something like this. On the server:
```ruby
register_presence_channel_prefix("site") do |channel|
next nil unless channel == "/site/online"
PresenceChannel::Config.new(public: true)
end
```
And on the client, a component could be implemented like this:
```javascript
import Component from "@ember/component";
import { inject as service } from "@ember/service";
export default Component.extend({
presence: service(),
init() {
this._super(...arguments);
this.set("presenceChannel", this.presence.getChannel("/site/online"));
},
didInsertElement() {
this.presenceChannel.enter();
this.presenceChannel.subscribe();
},
willDestroyElement() {
this.presenceChannel.leave();
this.presenceChannel.unsubscribe();
},
});
```
With this template:
```handlebars
Online: {{presenceChannel.count}}
<ul>
{{#each presenceChannel.users as |user|}}
<li>{{avatar user imageSize="tiny"}} {{user.username}}</li>
{{/each}}
</ul>
```
Uppy V2 includes the S3 multipart batch presigning change
we contributed in d613b849a6
so we need to upgrade it. This also brings both package.json
files into line and accounts for the renaming of Plugin
to BasePlugin in Uppy.
This has been tested and is working locally for both
regular Ember and Ember CLI, for uploads.json
XHR uploads and for direct S3 uploads (single and multipart).
This mixin needs to be shared between the composer and composer-like
user interfaces. This commit makes it so the events and the underlying
data model is configurable by the component extending the ComposerUploadUppy
mixin.
Also removes two MessageBus unsubscribe calls which were unnecessary.
The generate_presigned_put endpoint for direct external uploads
(such as the one for the uppy-image-uploader) records allowed
S3 metadata values on the uploaded object. We use this to store
the sha1-checksum generated by the UppyChecksum plugin, for later
comparison in ExternalUploadManager.
However, we were not doing this for the create_multipart endpoint,
so the checksum was never captured and compared correctly.
Also includes a fix to make sure UppyChecksum is the last preprocessor to run.
It is important that the UppyChecksum preprocessor is the last one to
be added; the preprocessors are run in order and since other preprocessors
may modify the file (e.g. the UppyMediaOptimization one), we need to
checksum once we are sure the file data has "settled".
The user-topic-list template is also in use in other places when we want to improve blank page syndrome, so this PR is a preparation for that changes as well.
- uses tagName=""
- removes user property which is not being used
- extract utility functions
- better wording for boolean properties
- initializes all properties
- uses @action
- uses optional chaining
- other minor changes
This rolls uppy back to the previous bundle that was used,
which will break multipart functionality (which is not yet
enabled anywhere).
No other upload functionality should be affected by this change,
it will be as if d295a16dab had
not been merged.
Users can invite people to topic and they will be automatically
redirected to the topic when logging in after signing up. This commit
ensures a "invited_to_topic" notification is created when the invite is
redeemed.
The same notification is used for the "Notify" sharing method that is
found in share topic modal.
This bug was introduced by f66007ec83.
In PostJobsEnqueuer we previously did not fire the after_post_create
event and after_topic_create event for private message topics. This was
changed in the above commit in order to publish message bus messages
for topic tracking state updates. Unfortunately this caused the
NotifyMailingListSubscribers job to be enqueued for all posts including
private messages, and admins and the users involved in the PMs got
emailed the contents of the PMs if they had mailing list mode enabled.
Luckily the impact of this was mitigated by a Guardian#can_see? check
for each mailing list mode user in the NotifyMailingListSubscribers job.
We never want to notify mailing list mode subscribers for private messages
so an early return has been added there, plus the logic in PostJobsEnqueuer
has been fixed, and tests have been added to that class where there were
none before.
Since ad3ec5809f when a user chooses
the Dismiss New... option in the New topic list, we send a request
to topics/reset-new.json with ?tracked=false as the only parameter.
This then uses Topic as the scope for topics to dismiss, with no
other limitations. When we do topic_scope.pluck(:id), it gets the
ID of every single topic in the database (that is not deleted) to
pass to TopicsBulkAction, causing a huge query with severe performance
issues.
This commit changes the default scope to use
`TopicQuery.new(current_user).new_results(limit: false)`
which should only use the topics in the user's New list, which
will be a much smaller list, depending on the user's "new_topic_duration_minutes"
setting.
Whenever we `subscribe` to something there should be an equivalent
`unsubscribe` and this implements it for `LogsNotice`.
In the future we should make this closer to what Ember expects a Service
to be, but at least it's properly cleaning up after itself now.
This can be used to change the list of topic posters. For example,
discourse-solved can use this to move the user who posted the solution
after the original poster.
See the previous commit d66b258b0e as
well.
If enable_upload_debug_mode is true, we do not want to abort the
direct S3 upload, because that will delete the file on S3 and prevent
further inspection of any errors that have come up.
There are certain design decisions that were made in this commit.
Private messages implements its own version of topic tracking state because there are significant differences between regular and private_message topics. Regular topics have to track categories and tags while private messages do not. It is much easier to design the new topic tracking state if we maintain two different classes, instead of trying to mash this two worlds together.
One MessageBus channel per user and one MessageBus channel per group. This allows each user and each group to have their own channel backlog instead of having one global channel which requires the client to filter away unrelated messages.
This pull request introduces the endpoints required, and the JavaScript functionality in the `ComposerUppyUpload` mixin, for direct S3 multipart uploads. There are four new endpoints in the uploads controller:
* `create-multipart.json` - Creates the multipart upload in S3 along with an `ExternalUploadStub` record, storing information about the file in the same way as `generate-presigned-put.json` does for regular direct S3 uploads
* `batch-presign-multipart-parts.json` - Takes a list of part numbers and the unique identifier for an `ExternalUploadStub` record, and generates the presigned URLs for those parts if the multipart upload still exists and if the user has permission to access that upload
* `complete-multipart.json` - Completes the multipart upload in S3. Needs the full list of part numbers and their associated ETags which are returned when the part is uploaded to the presigned URL above. Only works if the user has permission to access the associated `ExternalUploadStub` record and the multipart upload still exists.
After we confirm the upload is complete in S3, we go through the regular `UploadCreator` flow, the same as `complete-external-upload.json`, and promote the temporary upload S3 into a full `Upload` record, moving it to its final destination.
* `abort-multipart.json` - Aborts the multipart upload on S3 and destroys the `ExternalUploadStub` record if the user has permission to access that upload.
Also added are a few new columns to `ExternalUploadStub`:
* multipart - Whether or not this is a multipart upload
* external_upload_identifier - The "upload ID" for an S3 multipart upload
* filesize - The size of the file when the `create-multipart.json` or `generate-presigned-put.json` is called. This is used for validation.
When the user completes a direct S3 upload, either regular or multipart, we take the `filesize` that was captured when the `ExternalUploadStub` was first created and compare it with the final `Content-Length` size of the file where it is stored in S3. Then, if the two do not match, we throw an error, delete the file on S3, and ban the user from uploading files for N (default 5) minutes. This would only happen if the user uploads a different file than what they first specified, or in the case of multipart uploads uploaded larger chunks than needed. This is done to prevent abuse of S3 storage by bad actors.
Also included in this PR is an update to vendor/uppy.js. This has been built locally from the latest uppy source at d613b849a6. This must be done so that I can get my multipart upload changes into Discourse. When the Uppy team cuts a proper release, we can bump the package.json versions instead.
Steps to reproduce:
1. Go to activity/bookmarks
2. Search for something that isn’t in your bookmarks, so you get no results
3. Navigate away and then click "Bookmarked" on the sidebar or open the user menu and click the View All Bookmarks button on the bottom of the bookmarks tab, and you get the message "You haven't bookmarked anything yet".
This commit fixes the problem. We have a controller with a query parameter q that contains a search query. And we also have a property searchTerm that is bound to the search box on the page and mirrors the value in q. We were using a value from searchTerm when querying the server, but ember controllers are singletons so the searchTerm value persisted between page visits and leaded to this bug.
To make things work properly, we should be using the value from q everywhere except two places when we copy a value from q to searchTerm and vice versa.
Major changes included:
- better support for screen readers
- trapping focus in modals
- better tabbing order in composer
- alerts on no content found/number of items found
- better autofocus in modals
- mini-tag-chooser is now a multi-select component
- each multi-select-component will now display selection on one row
Plugin API is allowing to add small action codes dedicated to groups.
This will be used by assign-plugin when topic is assigned or unassigned from group.
When resetting the preprocessor status states, we weren't using
the same default state as when the preprocessor status state is
first initialized with an associated plugin. This commit brings
the two into alignment, fixing a bug where if you cancelled an
upload then tried a new one the "Processing Upload" message would
never change to "Uploading... X", so any subsequent uploads were
uncancellable.
Since the state was not being reset correctly, the properties that
were supposed to be numbers ended up as `undefined`, so when calling
prop-- or prop++, they turned into NaN.
This change only applies when uppy is calling the media-optimization-worker.
Since the old way of calling the worker via jQuery file uploader will
be removed soon, there is no point coming up with some random string
to use in place of the file name for the promise resolvers there, we
can live with this for now.
When we encountered an error with the media-optimization-worker,
we stopped the worker, which made it so further messages were not
received when optimizing images in parallel. Removed this based
on an option.
Also added more debugging lines to help track down issues.
* FIX: Revoking admin or moderator status doesn't require refresh to delete/anonymize/merge user
On the /admin/users/<id>/<username> page, there are action buttons that are either visible or hidden depending on a few fields from the AdminDetailsSerializer: `can_be_deleted`, `can_be_anonymized`, `can_be_merged`, `can_delete_all_posts`.
These fields are updated when granting/revoking admin or moderator status. However, those updates were not being reflected on the page. E.g. if a user is granted moderation privileges, the 'anonymize user' and 'merge' buttons still appear on the page, which is inconsistent with the backend state of the user. It requires refreshing the page to update the state.
This commit fixes that issue, by syncing the client model state with the server state when handling a successful response from the server. Now, when revoking privileges, the buttons automatically appear without refreshing the page. Similarly, when granting moderator privileges, the buttons automatically disappear without refreshing the page.
* Add detailed user response to spec for changed routes.
Add tests to verify that the revoke_moderation, grant_moderation, and revoke_admin routes return a response formatted according to the AdminDetailedUserSerializer.
When a theme's default color scheme is not marked as user selectable, we were outputting the numeric ID in the UI. This outputs "Theme default" instead.
This is useful in the DiscourseHub mobile app, currently the app queries
the `about.json` endpoint, which can raise a CORS issue in some cases,
for example when the site only accepts logins from an external provider.
I was storing the wrong object as the event listener
reference for the paste and mobile upload button click
events so they were not being cleaned properly on element
destruction.
Also renamed `uploadButton` to the more descriptive
`mobileUploadButton`.
When the composer reply is cancelled and the draft is trashed,
the isUploading and isProcessing statuses were not being reset,
so when the composer was opened again the Uploading... or
Processing... message still showed even when the uploads had
been cancelled correctly.
The regular composer-upload mixin suffered the same problem
as the uppy one, where the Processing/Uploading message was not
reset when a reply was cancelled and the draft destroyed.
When I added the paste event for files in the composer to
send to Uppy, I inadvertently called event.preventDefault()
if the pasted data was text. I removed that now, and I only
return early if the user cannot upload, and if there are no
files on the clipboard nothing happens.
Adds uppy upload functionality behind a
enable_experimental_composer_uploader site setting (default false,
and hidden).
When enabled this site setting will make the composer-editor-uppy
component be used within composer.hbs, which in turn points to
a ComposerUploadUppy mixin which overrides the relevant
functions from ComposerUpload. This uppy uploader has parity
with all the features of jQuery file uploader in the original
composer-editor, including:
progress tracking
error handling
number of files validation
pasting files
dragging and dropping files
updating upload placeholders
upload markdown resolvers
processing actions (the only one we have so far is the media optimization
worker by falco, this works)
cancelling uploads
For now all uploads still go via the /uploads.json endpoint, direct
S3 support will be added later.
Also included in this PR are some changes to the media optimization
service, to support uppy's different file data structures, and also
to make the promise tracking and resolving more robust. Currently
it uses the file name to track promises, we can switch to something
more unique later if needed.
Does not include custom upload handlers, that will come
in a later PR, it is a tricky problem to handle.
Also, this new functionality will not be used in encrypted PMs because
encrypted PM uploads rely on custom upload handlers.
When a staff member clicks on a user's number of flagged posts, we redirect them to the review queue, so it makes sense to count the number of items there to calculate the count.
We used to look at post action items to calculate this number, which doesn't match the number of items in the queue if old flags exist.
The invite acceptance page is an alternative signup flow, so it makes sense to include the new 'link' functionality there as well.
Followup to 7dc8f8b794
We've recently added a limit to the posts history modal so it displays the last 100 revisions only for performance reasons. However, the title of the modal now always says `History, last 100 revisions` even when the post has fewer than 100 revisions which can be a bit noisy.
This PR amends the history modal so the title of the modal says `History` when the post's revisions count is ≤100, and `History, last 100 revisions` when it has more >100 revisions.
When a user signs up via an external auth method, a new link is added to the signup modal which allows them to connect an existing Discourse account. This will only happen if:
- There is at least 1 other auth method available
and
- The current auth method permits users to disconnect/reconnect their accounts themselves
This handles a few edge cases which are extremely rare (due to the UI layout), but still technically possible:
- Ensure users are authenticated before attempting association.
- Add a message and logic for when a user already has an association for a given auth provider.
- active setting should now correctly show an arrow which was previously floating in the middle of nowhere
- uses a correct color for border separation, previously the border was present but invisible as similar to the background
- slighty tweak padding
- makes arrow computation based on a variable
When no element is selected, on the homepage for example, pressing `s` would generate the following error:
```
Uncaught TypeError: Cannot read property 'click' of undefined
```
Note that this commit also removes jquery usage.
This reverts a part of changes introduced by https://github.com/discourse/discourse/pull/13947
In that PR I:
1. Disallowed topic feature links for TL-0 users
2. Additionally, disallowed just putting any URL in topic titles for TL-0 users
Actually, we don't need the second part. It introduced unnecessary complexity for no good reason. In fact, it tries to do the job that anti-spam plugins (like Akismet plugin) should be doing.
This PR reverts this second change.
Plugins listening on the `before_auth` callback can interact with the request object and access data like the user agent or the remote IP address. We'll later store this data in the user record, but it might not exist at this point if we're authenticating a new account.
This adds an optional ENV variable, `EMBER_CLI_PROD_ASSETS`. If truthy,
compiling production assets will be done via Ember CLI and will replace
the assets Rails would otherwise use.
This disallows putting URLs in topic titles for TL0 users, which means that:
If a TL-0 user puts a link into the title, a topic featured link won't be generated (as if it was disabled in the site settings)
Server methods for creating and updating topics will be refusing featured links when they are called by TL-0 users
TL-0 users won't be able to put any link into the topic title. For example, the title "Hey, take a look at https://my-site.com" will be rejected.
Also, it improves a bit server behavior when creating or updating feature links on topics in the categories with disabled featured links. Before the server just silently ignored a featured link field that was passed to him, now it will be returning 422 response.
In the group interaction UI, if the default_notification_level for
a group was set to 0 (muted) it incorrectly showed as Watching in
the UI because of the ember or() helper, using JS comparison, considered
0 to be a falsey value and always showed 3 (watching) instead.
We are still on a version of pretender since 2017
https://github.com/pretenderjs/pretender/releases/tag/v1.6.1
Since then many changes have been made, including adding support
for xhr.upload. Upgrading will let us write proper acceptance
tests for uppy, which uses XmlHTTPRequest internally including
xhr.upload.
Updates pretender to 3.4.7 and fake-xml-http-request to 2.1.2.
Note: There have been no breaking changes in the releases that would
affect us, mainly dropping support for old node versions.
When declaring your widget you can now add an option like: `services: ['cool']`
And your widget instances will automatically get a `this.cool` property
which will resolve to the service. This saves having to look it up
yourself.
We rely on yarn workspaces so we don't want people using npm in the repo by accident.
Also updated the required node version to 12+.
~~Not sure about the min yarn version – the latest one could be missing in various CI-like envs, so I might change it yet.~~
Downgraded yarn to ">= 1.21.1" (the oldest of "current" versions, tagged "legacy")
* FIX: Update draft count when sequence is increased
Sometimes users ended up having a draft count higher than the actual
number of drafts.
* FIX: Do not update draft count twice
The call to DraftSequence.next! above already does it.
Group flair is not removed while removing a user from the group since the `before_save` callback methods are not triggered while using the `update_columns` method.
Currently when a user clicks on an edit notification, we use `appEvents` to
notify the topics controller that it should open up the history modal for the
edited post and the appEvents callback opens up the history modal in the next
Ember runloop (by scheduling an `afterRender` callback).
There are 2 problems with this implementation:
1) the callbacks are fired/executed too early and if the post has never been
loaded from the server (i.e. not in cache), we will not get a modal history
because the method that shows the modal `return`s if it can't find the post:
016efeadf6/app/assets/javascripts/discourse/app/controllers/topic.js (L145-L152)
2) when clicking an edit notification from a non-topic page, you're redirected
to the topic page that contains the edited post and you'll see the history
modal briefly and it'll be closed immediately. The reason for this is because
we attempt to show the history modal before the route transition finishes
completely, and we have cleanup code in `initializers/page-tracking.js` that's
called after every transition and it does several things one of which is
closing any open modals.
The fix in this commit defers showing the history modal until posts are loaded
(whether fresh or cached). It works by storing some bits of information (topic
id, post number, revision number) whenever the user clicks on an edit
notification, and when the user is redirected to the topic (or scrolled to the
edited post if they're already in the topic), the post stream model checks if
we have stored information of an edit notification and requests the history
modal to be shown by the topics controller.
Some authentication buttons (e.g. apple, oidc, oauth2, saml) do not have a specific color specified. Therefore they were taking the default button-with-icon color, and the icons would almost disappear on hover. This commit adds a default of #000 for these buttons, so that the button hover looks similar to core auth buttons.
This PR moves all the upload related functions into a new
ComposerUpload mixin that is extended by the composer-editor
component. This is being done so I can introduce a ComposerUploadUppy
mixin that overrides functions in the regular ComposerUpload mixin,
via a new composer-editor-uppy component that inherits from
ComposerEditor. The proposed structure, which will be in the next PR,
looks like this:
composer-editor-uppy
```javascript
import ComposerEditor from "discourse/components/composer-editor"
import ComposerUploadUppy from "discourse/mixins/composer-upload-uppy"
export default ComposerEditor.extend(ComposerUploadUppy, {
layoutName: "components/composer-editor"
});
```
This way the new composer-editor is a dumb component purely used for
testing uppy safely, and within the template for composer.hbs we do
this:
```javascript
@discourseComputed
composerComponent() {
return this.siteSettings.enable_experimental_composer_uploader
? "composer-editor-uppy"
: "composer-editor";
},
```
```handlebars
{{component composerComponent ...}}
```
This is the only way I can think to do it, because it is not possible to
access the site settings when the component is first declared I can't do
something like:
```javascript
const uploaderMixin = this.siteSettings.use_experimental_uploader?
ComposerUploaderUppy : ComposerUploader;
Component.extend(uploaderMixin, {});
```
An additional change in this PR is explicitly passing in these four
plugin data structures to the composer-editor Component, rather
than relying on JS closures which the mixin cannot do:
* uploadMarkdownResolvers
* uploadProcessorActions
* uploadProcessorQueue
* uploadHandlers
While merging two user accounts don't merge the source user's email address if the target user is not a human.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This PR adds a hover title to a few columns on the admin users' page
/admin/users/list/active
The hover title will show the date in full format on those columns with shortened dates
Searching in a category looked only one level down, ignoring the site
setting max_category_nesting. The user interface did not support the
third level of categories and did not display them in the "Categorized"
input of the advanced search options.
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.