Draft: Adds Action field to Notes Webhooks
As discussed previously on !127169 (comment 1827293547), this mr adds an action field to the note payload, thus informing the hook receiver what was the action of the note change.
Depends: !127169 (merged) Closes: https://gitlab.com/gitlab-com/account-management/eastern-north-america/red-hat/red-hat-kernel-team/-/issues/154
Merge request reports
Activity
@carlad-gl @jnutt This is the feature we discussed on my previous mr, the only thing that I'm a bit dubious about this change is, do we really need to create a
action
entry on notes? or should I just be passing the action as a parameter to NoteBuilder?Edited by Lucas ZampieriHey @lzampier! Thanks for working on this.
I don't think we need to record the value in the
Notes
table itself. We can include the value in the JSON payload depending on which code path triggered the hook.@carlad-gl mentioned that similar behaviour exists for merge requests, which we handle in
MergeRequests::BaseService
.One thing I'd like to flag is that the documentation states that there are some situations where we don't guarantee webhook uniqueness. Would this change still satisfy your use case if you happened to receive the same
create
hook payload twice?
Hey @lzampier!
Thank you for your contribution to GitLab. Please refer to the contribution documentation for an overview of the process.
Did you know about our community forks? Working from there will make your contribution process easier. Please check it out!
When you're ready for a first review, post
@gitlab-bot ready
. If you know a relevant reviewer(s) (for example, someone that was involved in a related issue), you can also assign them directly with@gitlab-bot ready @user1 @user2
.At any time, if you need help, feel free to post
@gitlab-bot help
or initiate a mentor session on Discord. Read more on how to get help.You can comment
@gitlab-bot label <label1> <label2>
to add labels to your MR. Please see the list of allowed labels in thelabel
command documentation.This message was generated automatically. You're welcome to improve it.
added Community contribution workflowin dev labels
assigned to @lzampier
5 Warnings This MR changes code in ee/
, but its Changelog commit is missing theEE: true
trailer. Consider adding it to your Changelog commits.This merge request is quite big (566 lines changed), please consider splitting it into multiple merge requests. 822a8013: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. New migrations added but db/structure.sql wasn't updated Usually, when adding new migrations, db/structure.sql should be
updated too (unless the migration isn't changing the DB schema
and isn't the most recent one).This merge request does not refer to an existing milestone. 1 Message This merge request adds or changes files that require a review from the Database team. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
db/migrate/20240322131302_add_action_field_to_notes.rb
Reviewer roulette
Category Reviewer Maintainer backend @praba.m7n
(UTC-7)
@schin1
(UTC+8)
database @johnmason
(UTC-4)
@praba.m7n
(UTC-7)
groupimport and integrate (backend) @jnutt
(UTC+0)
Maintainer review is optional for groupimport and integrate (backend) Please check reviewer's status!
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Dangermentioned in merge request !127169 (merged)
added 358 commits
-
b5be4b12...5a4fdbc9 - 357 commits from branch
gitlab-org:master
- a5f5a182 - Add Action field to Notes Webhook payload
-
b5be4b12...5a4fdbc9 - 357 commits from branch
mentioned in issue gitlab-org/quality/triage-reports#16909 (closed)
added groupimport and integrate label
added devopsmanage sectioncore platform labels
added backend featureaddition labels and removed devopsmanage sectioncore platform labels
added typefeature label
added devopsmanage sectioncore platform labels
@lzampier Thank you for starting this MR
I'll close this in favour of !147856 (merged).@lzampier, how was your code review experience with this merge request? Please tell us how we can continue to iterate and improve:
- React with a
or a on this comment to describe your experience. - Create a new comment starting with
@gitlab-bot feedback
below, and leave any additional feedback you have for us in the comment.
Subscribe to the GitLab Community Newsletter for contributor-focused content and opportunities to level up.
Thanks for your help!
This message was generated automatically. You're welcome to improve it.
- React with a