Separate reference gathering from rendering
This is a required step to allow batch processing when gathering references. This in turn would allow grabbing (for example) all mentioned users of an issue/merge request using a single query.
cc @rspeicher @DouweM
See merge request !3969
There are several changes to this module:
1. The use of an explicit stack in Participable#participants
2. Proc behaviour has been changed
3. Batch permissions checking
== Explicit Stack
Participable#participants no longer uses recursion to process "self" and
all child objects, instead it uses an Array and processes objects in
breadth-first order. This allows us to for example create a single
Gitlab::ReferenceExtractor instance and pass this to any Procs. Re-using
a ReferenceExtractor removes the need for running potentially many SQL
queries every time a Proc is called on a new object.
== Proc Behaviour Changed
Previously a Proc in Participable was expected to return an Array of
User instances. This has been changed and instead it's now expected that
a Proc modifies the Gitlab::ReferenceExtractor passed to it. The return
value of the Proc is ignored.
== Permissions Checking
The method Participable#participants uses
Ability.users_that_can_read_project to check if the returned users have
access to the project of "self" _without_ running multiple SQL queries
for every user.
In 8278b763d9 the default behaviour of annotation
has changes, which was causing a lot of noise in diffs. We decided in #17382
that it is better to get rid of the whole annotate gem, and instead let people
look at schema.rb for the columns in a table.
Fixes: #17382
It's unclear how these blank values got added, but GitLab.com had a few:
```
irb(main):002:0> Note.where("line_code IS NOT NULL AND line_code = ''").count
=> 439
```
We've added a migration to convert any existing records to use a NULL
value when blank, and updated Note to set blank values to nil before
validation.
These three tests were essentially checking that adding a note to
something updated its `noteable`'s `updated_at` attribute.
This is well-tested Rails behavior and we shouldn't feel the need to
write an integration test to verify it. At most we should be ensuring
that the association definition adds the `touch: true` option, which we
now do in Note's unit test.
write_ was renamed to create_
modify_ was renamed to update_
So now in update action we have next code
def create
can?(current_user, :create_issue, @issue)
end
def update
can?(current_user, :update_issue, @issue)
end
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
**What does this do?**
It makes sure that whenever a new note is added to an noteable item, the
updated_at of that item is also updated.
**Why is this needed?**
At this moment when you post a comment on an issue or add a label to an issue,
the updated_at is not changed. Because of this the filtering for least recently
updated is not really useful (since it only takes in account the original text
from the noteable).
Signed-off-by: Jeroen van Baarsen <jeroenvanbaarsen@gmail.com>
Douglas Barbosa Alexandre
ZJ van de Weg
Stan Hu
Douwe Maan
Yorick Peterse
Robert Speicher
Valery Sizov
Robert Speicher
Grzegorz BizonandRobert Speicher
Fatih Acet
Douwe Maan
Jeroen van Baarsen
Rémy Coutable
Dmitriy Zaporozhets