This new site setting replaces the
`enable_experimental_sidebar_hamburger` and `enable_sidebar` site
settings as the sidebar feature exits the experimental phase.
Note that we're replacing this without depreciation since the previous
site setting was considered experimental.
Internal Ref: /t/86563
When finding the candidates for `Topic.similar_to`, we will now ignore
topics in categories where `Category#search_priority` has been set to
ignore and also topics in categories which the user has specifically
muted.
Internal Ref: /t/87132
Previously we would trigger the event before the `Topic#deleted_at`
column has been updated making it hard for plugins to correctly work
with the model when its new state has not been persisted in the
database.
With the refactoring of the user messages routes in
4da2e3fef4, we can now depend on the top
level routes like `userPrivateMessages.user`, `userPrivateMessages.group` and `userPrivateMessages.tags`
to determine what the active value for the dropdown should be which
greatly simplifies the logic.
The centralization helps in reducing code duplication in our code base
and more importantly, centralizing logic for guardian checks into a
single spot.
Currently this is how the navigation structure looks like on the messages page:
#### When personal inbox route is active
```
Inbox
sent
new
unread
archive
Group 1 Inbox
Group 2 Inbox
Tags
<Plugin Outlet>
```
#### When group inbox route is active
```
Inbox
Group 1 Inbox
sent
new
unread
archive
Group 2 Inbox
Tags
<Plugin Outlet>
```
With the existing structure, it is very easy for plugins to add additional navigation links by using the plugin outlet. In the redesigned user page navigation, the navigation structure on the messages page has been changed to look like this:
#### When personal inbox route is active
```
---dropdown-------
| Inbox | Latest | Sent | New | Unread | Archive
------------------
```
#### When group inbox route is active
```
---dropdown------
| Group 1 Inbox | Latest | New | Unread | Archive
-----------------
```
With the new navigation structure, we can no longer rely on a simple plugin outlet to extend the navigation structure. Instead, we will need to introduce a plugin API for plugins to extend the navigation structure. The API needs to allow two things to happen:
1. The plugin API needs to allow the plugin to register an item in the drop down and for the registered item to be "selected" whenever the plugin's routes are active.
1. The plugin API needs to allow the plugin to register items into the secondary horizontal navigation menu beside the drop down.
While trying to design the API, I struggle with trying to determine the "context" of the current route. In order words, it was hard to figure out if the current user is viewing the personal inbox, group inbox or tags. This is attributed to the fact that our current routing structure looks like this:
```
this.route(
"userPrivateMessages",
{ path: "/messages", resetNamespace: true },
function () {
this.route("new");
this.route("unread");
this.route("archive");
this.route("sent");
this.route("warnings");
this.route("group", { path: "group/:name" });
this.route("groupArchive", { path: "group/:name/archive" });
this.route("groupNew", { path: "group/:name/new" });
this.route("groupUnread", { path: "group/:name/unread" });
this.route("tags");
this.route("tagsShow", { path: "tags/:id" });
}
);
```
In order to provide context of the current route, we currently require all child routes under the `userPrivateMessages` route to set a `pmView` property on the `userPrivateMessages` controller. If the route requires additional context like the group currently active on the group inbox routes, the child routes would then have to set the `group` property on the `userPrivateMessages` controller. The problems with this approach is that we end up with many permutations of state on the `userPrivateMessages` controller and have to always clean up the state when navigating between the child routes. Basically, data is flowing upwards from the child routes into the parent controller which is not an ideal approach because we cannot easily determine where the "data" setup happens. Instead, we want to follow something similar to the "Data down, actions up" pattern where data flows downwards. In this commit, the `userPrivateMessages` routes have been changed to look like this:
```
this.route(
"userPrivateMessages",
{ path: "/messages", resetNamespace: true },
function () {
this.route("user", { path: "/" }, function () {
this.route("new");
this.route("unread");
this.route("archive");
this.route("sent");
this.route("warnings");
});
this.route("group", { path: "group/:name" }, function () {
this.route("archive");
this.route("new");
this.route("unread");
});
this.route("tags", { path: "/tags" }, function () {
this.route("show", { path: ":id" });
});
}
);
```
Basically, we group the child routes based on the purpose each route servers. User inbox routes are grouped together while group inbox routes are grouped together. A big benefit of this is that now have a different Ember router and controller for each grouping of child routes. The context of the current route is then tied directly to the route name instead of requiring each child route to set an attribute on the parent controller.
The second reason for why we needed to group the child routes together is because it allows us to pass the responsibility of rendering the secondary navigation links to the child routes. In this commit, we use the `{{in-element}}` modifier in the child route to render the secondary navigation links.
```
---dropdown--------
| Group 1 Inbox | Latest | New | Unread | Archive
------------------------
<parent template> <horizontal secondary navigation links element>
```
This means that each child route with its own model and context can then handle the responsibility of rendering the secondary navigation links without having to pass its context up to the `userPrivateMessages` controller. While this should have simplified by the `userPrivateMessages` controller, we can't do that in this commit because our current navigation structure requires all links for all message inboxes to remain on screen at all times. Once we fully transition to the redesigned user menu navigation, we will be able to greatly simplify things around the routes and controllers for `userPrivateMessages`.
In an ideal world, we would deprecate the old routes but I have done a quick search through all known plugins and no plugins are currently relying on those routes. There is a chance we could break plugins here but I'll like to see some smoke first before committing to the effort of deprecating client side routes.
Fixes broken behaviour of arrow buttons for certain users as the interval to scroll menu can be cancelled before the scrolling actually happens.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
While load testing our user creation code path in production, we
identified that executing the DB statement to update the `Group#user_count` column within a
transaction is creating a bottleneck for us. This is because the
creation of a user and addition of the user to the relevant groups are
done in a transaction. When we execute the DB statement to update
`Group#user_count` for the relevant group, a row level lock is held
until the transaction completes. This row level lock acts like a global
lock when the server is creating users that will be added to the same
group in quick succession.
Instead of updating the counter cache within a transaction which the
default ActiveRecord `counter_cache` option does, we simply update the
counter cache outside of the committing transaction.
Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>
Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>
This change only affects the redesign user page navigation menu. The
dismiss button was incorrectly positioned below the user notifications
stream. A side effect of this is that it affected our infinite loading
code for the notifications stream.
No tests have been added in this commit as we have yet to quite figure
out how we can reliabily test for behaviours which requires us to scroll
the page.
When a client "reads" a post, we do no immediately send the data of the
post for processing on the server. Instead, read posts data is batched
together and sent to the server for processing at regular intervals. On
the server side, processing of read posts data is done in the
background. As such, there is a small window of delay before a post is
marked as read by a user on the server side.
If a client reads a topic and loads the messages topic list before the
server has processed the read post, the unread posts count for the topic
which the client just read will appear to be incorrect/outdated.
As part of tracking a post as read, we are already tracking the highest
read post number for the last read topic by the client. Therefore, we
can use this information to correct the highest post read number in the
scenario that was described above. This solution is the same as what
we've been doing for the regular topics list.
I'm hesitant to call this a performance improvement since claiming a
reviewable is probably rare. However, this commit cuts out two DB
queries each time we have to publish a reviewable claimed message. More
importantly, publishing to groups scales much better than publishing to
users because we esstentially cap the number ids we have to load into
memory.
While updating all user pages to use the new horizontal, scrollable user
page navigation, we've inadvertently broken the interface for plugins which rely on the
`user-main-nav` plugin outlet to extend the user profile page. Such
plugins usually add a new user profile page with the following
template structure which is copied from Discourse core before the user
page navigation redesign:
```
{{#d-section pageClass="..." class="user-secondary-navigation" scrollTop=false}}
{{#mobile-nav class="..." desktopClass="action-list nav-stacked"}}
...
{{/mobile-nav}}
{{/d-section}}
<section class="user-content">
{{outlet}}
</section>
```
This commit seeks to add backwards compatibility in terms of the styling
of the interface such that even if the old template structure is used,
it would not look completely broken.
Product has decided that the tracked section link provides very little
value at this moment in time so we're removing it.
See https://meta.discourse.org/t/245374 for more context.
The user preferences tracking page is only present when the redesign
user navigation menu is enabled. During the first pass of
implementation, some old bugs were introduced and this commit fixes
that. Regression tests have also been added.
There are two possible ordering for categories shown in sidebar with
this commit.
When the `fixed_category_positions` site setting is enabled, the
categories are ordered based on `Category#position` which is a configurable
option by the user. When said site setting is disabled, the categories
are ordered based on `Category#name`.
The categories in Sidebar are also sorted in such a way where child
categories are always ordered right after their parents. When multiple
child categories are present, the child categories are ordered based on
the ordering described above.
`siteSettings` is now a service which means there should only be one
state for `siteSettings` during the life time of the application. This
also helps to maintain parity with production where the `site` model
relies on the `siteSettings` service and not a clone of the attributes.
This commit fixes a bug on the client site where we would include the
`regular_category_ids` field when trying to update the notification levels of
categories for a user. The `regulary_category_ids` field should only be
included when the `mute_all_categories_by_default` is enabled
Before this commit, we did not have guardian checks in place to determine if a
topic's title associated with a user badge should be displayed or not.
This means that the topic title of topics with restricted access
could be leaked to anon and users without access if certain conditions
are met. While we will not specify the conditions required, we have internally
assessed that the odds of meeting such conditions are low.
With this commit, we will now apply a guardian check to ensure that the
current user is able to see a topic before the topic's title is included
in the serialized object of a `UserBadge`.
The previous sidebar default tags and categories implementation did not
allow for a user to configure their sidebar to have no categories or
tags. This commit changes how the defaults are applied. When a user is being created,
we create the SidebarSectionLink records based on the `default_sidebar_categories` and
`default_sidebar_tags` site settings. SidebarSectionLink records are
only created for categories and tags which the user has visibility on at
the point of user creation.
With this change, we're also adding the ability for admins to apply
changes to the `default_sidebar_categories` and `default_sidebar_tags`
site settings historically when changing their site setting. When a new
category/tag has been added to the default, the new category/tag will be
added to the sidebar for all users if the admin elects to apply the changes historically.
Like wise when a tag/category is removed, the tag/category will be
removed from the sidebar for all users if the admin elects to apply the
changes historically.
Internal Ref: /t/73500
Before this commit, there was no way for us to efficiently check an
array of topics for which a user can see. Therefore, this commit
introduces the `TopicGuardian#can_see_topic_ids` method which accepts an
array of `Topic#id`s and filters out the ids which the user is not
allowed to see. The `TopicGuardian#can_see_topic_ids` method is meant to
maintain feature parity with `TopicGuardian#can_see_topic?` at all
times so a consistency check has been added in our tests to ensure that
`TopicGuardian#can_see_topic_ids` returns the same result as
`TopicGuardian#can_see_topic?`. In the near future, the plan is for us
to switch to `TopicGuardian#can_see_topic_ids` completely but I'm not
doing that in this commit as we have to be careful with the performance
impact of such a change.
This method is currently not being used in the current commit but will
be relied on in a subsequent commit.
Building does not persist the object in the database which is
unrealistic since we're mostly dealing with persisted objects in
production.
In theory, this will result our test suite taking longer to run since we
now have to write to the database. However, I don't expect the increase
to be significant and it is actually no different than us adding new
tests which fabricates more objects.
Staged users are allowed to view topics they created in a read restricted category
when category has `Category#email_in` and
`Category#email_in_allow_strangers` configured.
Displays a sidebar section link to admin users when
`default_sidebar_categories` site setting has not been configured for the
site.
Internal Ref: /t/73500
On the server side, the only limitation for `Category#color` is a length
limit of 6. Therefore, we cannot assume on the client side that the hex
code is always 6 digits.
If a site has no default sidebar tags configured, show tags section if the user has personal sidebar tags configured.
Otherwise, hide the tags section from the sidebar for the user.
If a site has default sidebar tags configured, always display the tags section.
If a site has no default sidebar categories configured:
* Show categories section if user has categories configured
* Hide categories section if user does not have categories configured
If a site has default sidebar categories configured:
* Always show categories section
If there are no top tags that an anonymous user can see and the site do
not have default sidebar tags configured for anonymous users, hide the
tag section entirely.
The feedback we got here is that the contextual topic list link
behaviour is surprising and people prefer that the topic list sidebar
links always go to a consistent location.
Internal ref: /t/74666/16
No tests cause a missing icon regression is not the end of the world and
I don't feel like it will provide enough value as a regression test long
term.
`discovery.category` route respects the default view of the category as
configured by `Category#default_view`. `discovery.latestCategory` forces
the latest filter when viewing the category which is not what we want.
https://meta.discourse.org/t/239999
In a recent commit when adding the review section link, I moved to a
pattern where we allowed the section links to be refreshed after the
section has been constructed. However, we were not tearing down the old
section links when refreshing. This made me realise that refreshing
section links in a section is not a pattern I want to adopt since people
can easily forget to teardown. Instead, each section link should be
responsible for defining a teardown function for cleanup which will
always be called when the sidebar is removed.
Previously we used `Category#category_excerpt` but the excerpt keeps the
HTML entities around if present and we can't really display HTML in the
title of a link.
This commit introduces an icon to all links in the sidebar. If an icon has not been configured, we will fall back to a generic "link" icon. As part of this commit, we also standardised the size of each prefix to 20px by 20px and set a fix margin. This is to allow sufficient space for text prefixes and image prefixes to be displayed.
Tests have been intentionally left out for now as I don't feel like asserting for the icons will bring much value at this point. Time shall prove me wrong.
Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>
When there are pending reviewables, the review section link is displayed
in the main section. When there are no pending reviewables, the review
section link is displayed under the more links drawer.
Internal ref: /t/74210
When `allow_uncategorized_topics` is set to `false`, we do not want to
show the uncategorized in sidebar by default.
This commit updates a couple of places in the code related to sidebar
which was incorrectly using `suppress_uncategorized_badge` site setting
which is mainly used for hiding the category badge for uncategorized
category and should not be used to determine if uncategorized categories
should be allowed or not.
This commits introduces a new SiteSetting.enable_new_user_profile_nav_groups
feature flag. When configured, users of the configured groups will see
the new user page navigation links.
As of this commit, only the user activity navigation link has been
converted to the newly proposed dropdown of navigation links.
Mobile support has not been considered.
In eb12daa7f8 when adding community
section support for anonymous users, we changed the `sectionLinks`
property into a getter method. This meant that if the getter method was
called again after the community section has been rendered, we would end
up reintializing the section links classes. As part of the
initialisation, some section links would setup a TopicTrackingState
onStateChange listener. However, the listener is only removed when the
entire community section is removed which resulted in us leaking the
onStateChange listeners.
This commit reverts the `sectionLinks` from being defined as a getter
method into a property which is only set once when the community section
is being constructor. Also, we changed it such that the community
section will register the listener instead of each section link since it
makes cleaning up much easier to reason about.
No tests have been added for this commit because the original bug is
not possible after this change and we already have an existing tests
ensuring that TopicTrackingState change listeners are cleaned up when
the community section is destroyed.
Internal ref: /t/73224
Alan Guo Xiang Tan
