Prior to this fix the state change was not reflected in the UI when changing the "who can assign" preference of a group.
This was actually saved in the backend but would require a full page refresh to see the updated state on the group preferences page.
At the moment, it's possible to have maximum 5 assignments per topic
(that includes topic and post assignments). When trying to assign more,
a message "Limit of 5 assignments per topic has been reached" appears.
One possible edge case here is _reassigning_ a topic or a post. Reassignment
doesn't lead to exceeding the limit, and therefore it should be possible. But
at the moment we handle correctly only _topic_ reassignments, while when
reassigning a _post_, we show the error message "the limit has been reached".
This commit makes post reassignments work correctly too.
Currently, when reopening a topic that has assignments, the
notifications in the user menu aren’t recreated.
This patch fixes that issue. It also addresses the same type of issue
with posts being destroyed and recovered.
Why this change?
We have been getting flaky test failures from these specs and the
failure screenshot shows that the user is not logged in when it is
supposed to be. Futher investigation shows that when the test flakes, it
is because the request to view the topic is using an auth token that is
different from the one which was created when the user was signed in.
What does this change do?
1. Add the `capture_log` metadata to all the tests in this file.
2. Enables the `verbose_auth_token_logging` site setting to give us more
debugging information in the logs.
Why this change?
The spec is flaky and it seems to be DB transaction related where after
signing in as a user, the failure screenshot shows that the user has not
been signed in.
What does this change do?
Set `capture_log: true` which will log all ActiveRecord DB statements
execute while running the spec.
Currently, when only one user is available for the random auto-assign
and if that user was already assigned, then the assign will fail.
This patch addresses this issue by allowing to reassign a user who’s
already assigned. A new parameter (`allow_self_reassign`) has been added
to `Assigner#assign` with a default value of `false`.
Sometimes, the topic id actually exists for that spec (id being 1) and
it will raise about not finding the group id instead of not finding the
provided topic because the assignees group wasn’t provided in the
`fields` parameter.
Currently, when the auto-assign logic can’t find a user to assign, it
will fail saying there was no one to assign. The current logic is this
one:
- Don’t pick anyone who’s been picked in the last 180 days
- If no one has been found, then try the same thing but only for the
last 14 days.
While this is working relatively well for large enough groups, it
doesn’t work at all with very small groups (like 2 people) and it
creates unnecessary noise.
This patch addresses this issue by adding a fallback to the current
logic. Now, if the two first rules fail, instead of saying that no one
was assigned, we assign the least recently assigned person. This way,
the logic will continue to work with large groups but will also work
nicely with small groups.
The recent changes to the user menu changed how we were ordering the
items in it. A small bug was noticed though, as we’re not displaying the
unread assignments at the very top.
This patch fixes the ordering like this:
- Unread individual assignment notifications
- Unread group assignment notifications
- Read individual assignments
- Read group assignments
In each group of items, they are sorted by date, the most recent ones
being at the top.
Currently, we display a mix of topics and notifications in the user menu
assignments tab. This has a number of issues like having to maintain
hard to understand code instead of simply relying on the notifications
system we have, we can’t display more than one assignment per topic and
it’s not clear if an assignment is for a topic or a post nor if it’s a
group assignment or an individual one.
This patch addresses those issues by relying on the notifications system
we’re using for most of the other user menu tabs instead of a custom
implementation. This led to some heavy refactoring but it was
worthwhile as things are a bit more normalized and easier to reason
about. Instead of serializing topics with different attributes to access
various assignments, we now simply return a notification for each
existing assignment.
The UI changed a bit: tooltips are now explaining if the assignment is
for a topic or a post and if it’s for an individual or a group. Icons
are also properly set (so no more individual icon for group assignments)
and the assigned group name is displayed.
The background jobs signatures changed a bit (`assignment_id` is now
needed for the unassign job) so when deploying this patch, it’s expected
to have some jobs failing. That’s why a post-migration has been written
to handle the creation of missing notifications and the deletion of
extra notifications too.
Why this change?
We have been seeing tests failures due to errors like
```
Capybara::ElementNotFound:
Unable to find css "#topic-footer-button-assign"
```
when running our system tests in CI. However, I can't quite figure out
why that is happening and have invested way too much time to do so.
Therefore, I'm trying to fix this by closing a topic via the topic
timeline controls instead of the footer button instead.
This was causing the tests to be flaky with the following errors on CI:
```
PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_assignments_on_target_id_and_target_type"
DETAIL: Key (target_id, target_type)=(39, Topic) already exists.
```
Otherwise, when an allowed group member is assigned to the PM it invites the user (which is unnecessary) and creates a public small post for the invite.
The goal is to add a class so when assigns are not public, their descriptions can be styled similar to whispers... this is a light way to reassure admins of assign message visibility.
When assignment data is preloaded, cache variable should be set when topic is assigned but also when topic is not assigned to avoid additional queries.
The previous behavior was:
1. Assign a topic to someone
2. Close the topic with the setting unassign_on_close enabled
3. Assign the topic to the same user as before
This caused small broken or empty posts to be displayed.
This commit also introduces system specs
This spec has been detected to be flaky by
https://github.com/discourse/flaky-tests-detective which we run
internally. However, I could not reproduce the flakiness after multiple
attempts so I rewrote the assertions such that we set up the
expectations of what `Assignment#assigned_to_id` should be before and
after the triggering of the DiscourseEvent.
If a topic is initially assigned to user A then reassigned to user B,
this change will make it so that notifications sent to user A are
removed. This matches the plugin's behaviour when unassigning a topic.
There is almost no reason for us to use `Fabricate.build` which does not persist the record to the database and is not what we encounter in production.
Joffrey JAFFEUX
Andrei Prigorshnev
Blake Erickson
Loïc Guitaut
Alan Guo Xiang Tan
David Taylor
Natalie Tay
Jarek Radosz
Krzysztof Kotlarek
Martin Brennan
Vinoth Kannan
Ted Johansson
Arpit Jalan
Bianca Nenciu
Kris
Jan Cernik
Gerhard Schlager
Penar Musaraj