Users can apply multiple suggestions at once.
What does this MR do?
This MR would fulfill the feature described in #25486 (closed) allowing users to apply multiple merge request suggestions at once.
Screenshots
Closes #25486 (closed)
Merge request reports
Activity
mentioned in issue #25486 (closed)
mentioned in issue #195097 (closed)
added Community contribution label
added devopscreate groupsource code labels
Thank you for this @jessehall3! Also ccing @andr3 due to the amount of frontend work.
Thank you @jessehall3!!!
added merge requests label
mentioned in merge request gitlab-com/www-gitlab-com!38274 (merged)
added 1769 commits
-
d6fed432...e3fae047 - 1768 commits from branch
gitlab-org:master
- 0712b5c6 - Can accept multiple suggestions at once.
-
d6fed432...e3fae047 - 1768 commits from branch
added 1256 commits
-
0650b9c8...ef172552 - 1248 commits from branch
gitlab-org:master
- 5f8b9767 - Can accept multiple suggestions at once.
- dce16d10 - initial tests passing in apply_service_spec
- 9c028e03 - All previous tests passing in apply_service_spec
- 7f3cb25b - Update to logic for applying batches -- now properly applying multiple suggestions on single file.
- 1373545a - New tests added to suggestions_spec
- a1491b55 - Start of work on updating frontend tests
- 16d8ce57 - Fixes for rubocop.
- e9bf6f12 - fixes for eslint
Toggle commit list-
0650b9c8...ef172552 - 1248 commits from branch
added 356 commits
-
e9bf6f12...0cf1bca0 - 355 commits from branch
gitlab-org:master
- 94eb9cd1 - Can apply suggestions in batches.
-
e9bf6f12...0cf1bca0 - 355 commits from branch
added documentation label
added 12 commits
- 892c2c64 - Changes to variable names.
- b5503b17 - can add and remove suggestions from a batch
- 4265c812 - Can apply multiple suggestions through the front end.
- 5dfdaaaf - Can get loading spinner when applying batch suggestions.
- b94bf3de - Now clearing batch after applying suggestion batch.
- b569cb95 - Refactoring methods
- b424a844 - added capybara tests
- e996dcb2 - updated temporary variable names
- a900c292 - added a couple of tests for the new actions. still need to the the submit batch action.
- f8bdbf82 - Passiing first test for submitSuggestionBatch action.
- 0547be83 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 9c7ec7ec - Linting updates.
Toggle commit listadded 1052 commits
-
9c7ec7ec...2e33d3e6 - 1038 commits from branch
gitlab-org:master
- 01ee42f0 - Can apply suggestions in batches.
- 27d783c9 - Updates to tests in apply_service_spec
- 957acaa2 - Changes to variable names.
- dd6a47a0 - can add and remove suggestions from a batch
- daa75046 - Can apply multiple suggestions through the front end.
- af945a12 - Can get loading spinner when applying batch suggestions.
- ef10a5b5 - Now clearing batch after applying suggestion batch.
- 61e00041 - Refactoring methods
- c4e270fe - added capybara tests
- 88529dd5 - updated temporary variable names
- a08a79c4 - added a couple of tests for the new actions. still need to the the submit batch action.
- 38273219 - Passiing first test for submitSuggestionBatch action.
- 38d0d25f - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 39323d83 - Linting updates.
Toggle commit list-
9c7ec7ec...2e33d3e6 - 1038 commits from branch
- Resolved by Bob Van Landuyt
I am ready for feedback on this MR. The functionality is in place and it passes the current test suite. As I highlighted above, I ran into some buggy behavior that may be a full blocker at this point. I'll wait to hear back on any next steps, thanks.
Edited by Jesse Hall
assigned to @dskim_gitlab
added 873 commits
-
901ae6ad...96fe039d - 858 commits from branch
gitlab-org:master
- 0954c8f8 - Can apply suggestions in batches.
- 08f6384c - Updates to tests in apply_service_spec
- dd0f2a32 - Changes to variable names.
- 8841fa49 - can add and remove suggestions from a batch
- 3bd7189b - Can apply multiple suggestions through the front end.
- 04f11a4f - Can get loading spinner when applying batch suggestions.
- af8b297d - Now clearing batch after applying suggestion batch.
- d9753553 - Refactoring methods
- c97c9ecd - added capybara tests
- 6bc329ee - updated temporary variable names
- 8d917d45 - added a couple of tests for the new actions. still need to the the submit batch action.
- 04e016ef - Passiing first test for submitSuggestionBatch action.
- b965d948 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 9a7fb017 - Updates to tests.
- aaf10503 - Updates to vue components and tests.
Toggle commit list-
901ae6ad...96fe039d - 858 commits from branch
added 69 commits
-
aaf10503...6f2abe2f - 54 commits from branch
gitlab-org:master
- bbc365a8 - Can apply suggestions in batches.
- c1fc8004 - Updates to tests in apply_service_spec
- 2f07a119 - Changes to variable names.
- 70ff0023 - can add and remove suggestions from a batch
- b7aa72e1 - Can apply multiple suggestions through the front end.
- 4979a09f - Can get loading spinner when applying batch suggestions.
- 6dd8f874 - Now clearing batch after applying suggestion batch.
- 85daf51f - Refactoring methods
- 8786aef5 - added capybara tests
- 1171a0b1 - updated temporary variable names
- a8a13650 - added a couple of tests for the new actions. still need to the the submit batch action.
- f9bc145f - Passiing first test for submitSuggestionBatch action.
- 25503e56 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 0866402f - Updates to tests.
- 1afe6fca - Updates to vue components and tests.
Toggle commit list-
aaf10503...6f2abe2f - 54 commits from branch
added 17 commits
-
1afe6fca...92c62ed1 - 2 commits from branch
gitlab-org:master
- 279f197b - Can apply suggestions in batches.
- f481b5ac - Updates to tests in apply_service_spec
- c009c943 - Changes to variable names.
- bc0a1163 - can add and remove suggestions from a batch
- 1e577c3a - Can apply multiple suggestions through the front end.
- d1eb9f22 - Can get loading spinner when applying batch suggestions.
- fc382030 - Now clearing batch after applying suggestion batch.
- c03f6b07 - Refactoring methods
- c71910d6 - added capybara tests
- 498097ed - updated temporary variable names
- accc31d3 - added a couple of tests for the new actions. still need to the the submit batch action.
- 1e7a4bc1 - Passiing first test for submitSuggestionBatch action.
- 8cafb153 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- ba910196 - Updates to tests.
- b5135074 - Updates to vue components and tests.
Toggle commit list-
1afe6fca...92c62ed1 - 2 commits from branch
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
- Resolved by Bob Van Landuyt
assigned to @jessehall3 and unassigned @dskim_gitlab
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Marcia Ramos
- Resolved by Bob Van Landuyt
Thanks @jessehall3! I've done the 1st pass of the docs review, please assign me for a second pass when ready.
I also looked at the UI text and suggested changing "appliable" to "applicable". I saw pieces of code using
appliable
and we don't have to change those, I'm only suggesting to change the text outputted to the user.
added Technical Writing UI text docsfeature labels
I guess we should ping a UXer here for review too?
cc/ @pedromsmentioned in issue #23718 (closed)
added 1 commit
- 93cc880c - New api endpoint for applying suggestion batches
assigned to @pedroms
- Resolved by Bob Van Landuyt
added 1 commit
- 79936d31 - Was not hiding loading spinner after failing to apply batch.
added 1 commit
- fa374c6a - Was not hiding loading spinner after failing to apply batch.
added 1577 commits
-
3ec98584...e8fb1e9f - 1557 commits from branch
gitlab-org:master
- 74b627f7 - Can apply suggestions in batches.
- 8f4a57ca - Updates to tests in apply_service_spec
- 8dfdab9c - Changes to variable names.
- fabf4077 - can add and remove suggestions from a batch
- 1a3f494f - Can apply multiple suggestions through the front end.
- 47473714 - Can get loading spinner when applying batch suggestions.
- a3369a50 - Now clearing batch after applying suggestion batch.
- dece0415 - Refactoring methods
- ee386841 - added capybara tests
- 19df72b7 - updated temporary variable names
- 307e0c6b - added a couple of tests for the new actions. still need to the the submit batch action.
- 47ac193b - Passiing first test for submitSuggestionBatch action.
- 1bf41cc3 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- b5aaa14b - Updates to tests.
- ef062a3a - Updates to vue components and tests.
- 46216f57 - code review revisions
- 96264527 - New api endpoint for applying suggestion batches
- 042fd602 - Updates to docs and error messages.
- 72b4c464 - Was not hiding loading spinner after failing to apply batch.
- 5bc67826 - Updates to docs and default commit message.
Toggle commit list-
3ec98584...e8fb1e9f - 1557 commits from branch
added 91 commits
-
5bc67826...b340fa7b - 71 commits from branch
gitlab-org:master
- 9e05a129 - Can apply suggestions in batches.
- 438601f2 - Updates to tests in apply_service_spec
- 7eb713c3 - Changes to variable names.
- 2e9f7656 - can add and remove suggestions from a batch
- 21eca884 - Can apply multiple suggestions through the front end.
- 779d82a9 - Can get loading spinner when applying batch suggestions.
- 7a1e8573 - Now clearing batch after applying suggestion batch.
- 1dc9a3f0 - Refactoring methods
- eca7deed - added capybara tests
- 02403341 - updated temporary variable names
- 92d11b7f - added a couple of tests for the new actions. still need to the the submit batch action.
- bc1bf915 - Passiing first test for submitSuggestionBatch action.
- 8fa86433 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 68d6ff27 - Updates to tests.
- 3cef32fb - Updates to vue components and tests.
- b74caad0 - code review revisions
- 0d8242a3 - New api endpoint for applying suggestion batches
- 2f3a56c9 - Updates to docs and error messages.
- a8f2d044 - Was not hiding loading spinner after failing to apply batch.
- c98b023b - Updates to docs and default commit message.
Toggle commit list-
5bc67826...b340fa7b - 71 commits from branch
added 317 commits
-
1cbc7398...09233b18 - 296 commits from branch
gitlab-org:master
- 55ce6c89 - Can apply suggestions in batches.
- 18d18a0f - Updates to tests in apply_service_spec
- 808913cd - Changes to variable names.
- bd52be24 - can add and remove suggestions from a batch
- 3f14312b - Can apply multiple suggestions through the front end.
- 3ff8b7eb - Can get loading spinner when applying batch suggestions.
- 7ceb7394 - Now clearing batch after applying suggestion batch.
- 926ed029 - Refactoring methods
- 67a9d254 - added capybara tests
- 8b1695ab - updated temporary variable names
- 7b32e9a8 - added a couple of tests for the new actions. still need to the the submit batch action.
- a043c408 - Passiing first test for submitSuggestionBatch action.
- 60068ee8 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 0755c7e2 - Updates to tests.
- 6ca4df8f - Updates to vue components and tests.
- 10cf1746 - code review revisions
- dc7244b3 - New api endpoint for applying suggestion batches
- f1032b77 - Updates to docs and error messages.
- 896eb60e - Was not hiding loading spinner after failing to apply batch.
- d1cfba8b - Updates to docs and default commit message.
- 7fdab647 - Refactoring apply_service.rb
Toggle commit list-
1cbc7398...09233b18 - 296 commits from branch
- Resolved by Pedro Moreira da Silva
- Resolved by Pedro Moreira da Silva
- Resolved by Pedro Moreira da Silva
- Resolved by Mark Florian
unassigned @pedroms
added 646 commits
-
7d4f5c83...ed3f5ff1 - 623 commits from branch
gitlab-org:master
- 1c4d3a49 - Can apply suggestions in batches.
- d5222279 - Updates to tests in apply_service_spec
- 12a777c0 - Changes to variable names.
- e3f06933 - can add and remove suggestions from a batch
- 00c9b827 - Can apply multiple suggestions through the front end.
- dca393f8 - Can get loading spinner when applying batch suggestions.
- f00f5019 - Now clearing batch after applying suggestion batch.
- 6290bb1d - Refactoring methods
- c3d52529 - added capybara tests
- cff5c844 - updated temporary variable names
- 865dd393 - added a couple of tests for the new actions. still need to the the submit batch action.
- 3d16ed70 - Passiing first test for submitSuggestionBatch action.
- fa656621 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 078003e4 - Updates to tests.
- 2e851b30 - Updates to vue components and tests.
- 42879cf6 - code review revisions
- 2ba14872 - New api endpoint for applying suggestion batches
- 238eebc4 - Updates to docs and error messages.
- a0ac02a3 - Was not hiding loading spinner after failing to apply batch.
- f64143ad - Updates to docs and default commit message.
- 875166f5 - Refactoring apply_service.rb
- 18f3fccd - Additional refactoring
- d36fbe78 - Updates after latest review
Toggle commit list-
7d4f5c83...ed3f5ff1 - 623 commits from branch
added 46 commits
-
7140a26d...cf7dca2b - 23 commits from branch
gitlab-org:master
- 2fee6686 - Can apply suggestions in batches.
- 281edc0f - Updates to tests in apply_service_spec
- 12122c85 - Changes to variable names.
- fb0af654 - can add and remove suggestions from a batch
- af875494 - Can apply multiple suggestions through the front end.
- c010afcd - Can get loading spinner when applying batch suggestions.
- 77b03564 - Now clearing batch after applying suggestion batch.
- 2f10c4bf - Refactoring methods
- c395b5c0 - added capybara tests
- e486c435 - updated temporary variable names
- 3e51fb0a - added a couple of tests for the new actions. still need to the the submit batch action.
- c5cd0ebe - Passiing first test for submitSuggestionBatch action.
- 6a119591 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 428c3a29 - Updates to tests.
- f80e1876 - Updates to vue components and tests.
- 42aafadf - code review revisions
- c068125e - New api endpoint for applying suggestion batches
- 3f6c91f1 - Updates to docs and error messages.
- 0aadf37c - Was not hiding loading spinner after failing to apply batch.
- b13b016e - Updates to docs and default commit message.
- aad66a9c - Refactoring apply_service.rb
- 452a60aa - Additional refactoring
- e279158b - Updates after latest review
Toggle commit list-
7140a26d...cf7dca2b - 23 commits from branch
- Resolved by Sincheol (David) Kim
- Resolved by Sincheol (David) Kim
added 481 commits
-
c0eac272...9fc8ba36 - 455 commits from branch
gitlab-org:master
- f9a0b332 - Can apply suggestions in batches.
- 6ab60eb5 - Updates to tests in apply_service_spec
- a4622956 - Changes to variable names.
- b0727aeb - can add and remove suggestions from a batch
- 1f3f4d73 - Can apply multiple suggestions through the front end.
- 1b78e5f1 - Can get loading spinner when applying batch suggestions.
- 9ae6352f - Now clearing batch after applying suggestion batch.
- 6e7abe76 - Refactoring methods
- 77a30ba6 - added capybara tests
- 8d4cb4e8 - updated temporary variable names
- b77dfd45 - added a couple of tests for the new actions. still need to the the submit batch action.
- f5d85f93 - Passiing first test for submitSuggestionBatch action.
- c36c4466 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 31457e0c - Updates to tests.
- 7148174f - Updates to vue components and tests.
- 615185b2 - code review revisions
- 0faafbbf - New api endpoint for applying suggestion batches
- eebf0106 - Updates to docs and error messages.
- 7005f563 - Was not hiding loading spinner after failing to apply batch.
- fd9e8214 - Updates to docs and default commit message.
- 721cd74a - Refactoring apply_service.rb
- ce349a39 - Additional refactoring
- 859361fb - Updates after latest review
- e7f7c35b - Changes following additional review
- 59766a4f - Missing green text within badge pill.
- 1e96a6fc - Added check for empty batches in api/suggestions.rb
Toggle commit list-
c0eac272...9fc8ba36 - 455 commits from branch
added 1 commit
- 632ca897 - Added check for empty batches in api/suggestions.rb
added 1 commit
- 6816559c - Added check for empty batches in api/suggestions.rb
assigned to @reprazent
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
Cool feature @jessehall3! I've left a bigger suggestion on how we could make the service class a bit thinner, let me know what you think.
I think the API isn't going to change much anymore, that looks pretty good, so I'd be fine starting the forntend review at the same time @andr3, who could help with that?
unassigned @reprazent
added 1 commit
- 619873bd - Fix for weird transition visuals between "applying" and "applied" states.
added 1 commit
- e514cec3 - Fix for weird transition visuals between "applying" and "applied" states.
@jboyson do you have capacity to review the FE here?
- Resolved by Bob Van Landuyt
@pedroms I can get to this tomorrow the Mar 13 if that works.
added 1 commit
- 60099054 - Split apply_service responsibilities into different classes.
added 1042 commits
-
60099054...6a4ed940 - 1014 commits from branch
gitlab-org:master
- 350b6b4d - Can apply suggestions in batches.
- 27feed12 - Updates to tests in apply_service_spec
- ab70ce8e - Changes to variable names.
- 97f65598 - can add and remove suggestions from a batch
- ddeda9f2 - Can apply multiple suggestions through the front end.
- bee432fc - Can get loading spinner when applying batch suggestions.
- 5c05804b - Now clearing batch after applying suggestion batch.
- 99439a46 - Refactoring methods
- e405f9d3 - added capybara tests
- dfa70529 - updated temporary variable names
- 98763ccd - added a couple of tests for the new actions. still need to the the submit batch action.
- 14631cbe - Passiing first test for submitSuggestionBatch action.
- df5d4530 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 90d2b0c4 - Updates to tests.
- 0d257836 - Updates to vue components and tests.
- 8491c0a4 - code review revisions
- 49570ad5 - New api endpoint for applying suggestion batches
- 3aaa5c3c - Updates to docs and error messages.
- a06ab248 - Was not hiding loading spinner after failing to apply batch.
- ffc7ab81 - Updates to docs and default commit message.
- 55af708d - Refactoring apply_service.rb
- 18bcb614 - Additional refactoring
- f915231c - Updates after latest review
- 87bcbdec - Changes following additional review
- 05e1f711 - Missing green text within badge pill.
- 546610b1 - Added check for empty batches in api/suggestions.rb
- e97f2086 - Fix for weird transition visuals between "applying" and "applied" states.
- 1cf924d5 - Split apply_service responsibilities into different classes.
Toggle commit list-
60099054...6a4ed940 - 1014 commits from branch
added 1 commit
- f1f61188 - Split apply_service responsibilities into different classes.
added 1 commit
- d4bdbd07 - Split apply_service responsibilities into different classes.
- Resolved by Justin Boyson
For frontend review considerations:
There is an issue I've found in both the current master branch and this fork that I would like to highlight. The behavior can be seen in the video clips below:
Master Branch master_apply_button_jumps
This Fork fork_apply_batch_button_jumps
After clicking either of the apply buttons and presenting the spinner, sometimes the spinner will jump back to showing an apply button before showing the final, green, "Applied" button.
I don't see this behavior all the time, which made capturing it on video difficult. Likewise, I have not see this on Gitlab.com, though I don't see why it should behave differently given that this behavior should rest solely on the logic on the frontend code.
I've tried different combinations of changes to the vue template, combined with tweaks to the vuex action that controls this, but I was unable to find a solution. Given the current logic in the template, I don't believe this behavior should be possible.
All that said, I wanted to be sure to state this here so that it can be understood as known behavior. Because I see it happen currently on the master branch, I would not consider it a blocker. However, I would understand if a reviewer feels that it requires further investigation before moving forward.
added 1 commit
- 0683c7fd - Resotres last commit id parameter for multi-service actions.
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
@jessehall3 Awesome work! I've left some comments, and a suggestion to get around the N+1. If that doesn't work we'll have to either implement a new RPC to get the last commit for multiple paths, or limit the number of different files we can apply suggestions to at one.
added 660 commits
-
852ba3e3...5f46d0b1 - 630 commits from branch
gitlab-org:master
- 25cd8364 - Can apply suggestions in batches.
- 410b83ae - Updates to tests in apply_service_spec
- 397fa35f - Changes to variable names.
- 3b90c245 - can add and remove suggestions from a batch
- 2e26cdf7 - Can apply multiple suggestions through the front end.
- aab4ea49 - Can get loading spinner when applying batch suggestions.
- 33a8c828 - Now clearing batch after applying suggestion batch.
- 23e989b2 - Refactoring methods
- d4db6d68 - added capybara tests
- e207aef5 - updated temporary variable names
- bd84efbd - added a couple of tests for the new actions. still need to the the submit batch action.
- 03ec12d7 - Passiing first test for submitSuggestionBatch action.
- ff3154bd - Updated logic in submitSuggestionsBatch action. updated note actions test.
- 56b3e352 - Updates to tests.
- 5ebbfecd - Updates to vue components and tests.
- f0c4ceb9 - code review revisions
- e2995a3a - New api endpoint for applying suggestion batches
- 52d73508 - Updates to docs and error messages.
- 42109af0 - Was not hiding loading spinner after failing to apply batch.
- 3a682eff - Updates to docs and default commit message.
- 71810828 - Refactoring apply_service.rb
- fd5e5e91 - Additional refactoring
- 68aa4430 - Updates after latest review
- fad02ee2 - Changes following additional review
- 591b100c - Missing green text within badge pill.
- 54def701 - Added check for empty batches in api/suggestions.rb
- b5298f2c - Fix for weird transition visuals between "applying" and "applied" states.
- cfc40a43 - Split apply_service responsibilities into different classes.
- bd922488 - Resotres last commit id parameter for multi-service actions.
- 2ef56a0c - code review changes
Toggle commit list-
852ba3e3...5f46d0b1 - 630 commits from branch
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
@jessehall3 Excellent work, the comments are more nitpicky in nature now, which means we're getting close, main things are the way we look up a commit id, and the separation of responsibility in tests. Let me know what you think.
@jessehall3 is this ready for another review? Wasn't 100% sure if you applied all suggestions
Thanks for checking up on this but, no, this is not ready for another review. I will ping @reprazent when it is ready.
added 1247 commits
-
955f9deb...b6b8ee4e - 1216 commits from branch
gitlab-org:master
- 9b99fd7c - Can apply suggestions in batches.
- 77e22db7 - Updates to tests in apply_service_spec
- 18748593 - Changes to variable names.
- b2679813 - can add and remove suggestions from a batch
- 74fb70f2 - Can apply multiple suggestions through the front end.
- 01b1b339 - Can get loading spinner when applying batch suggestions.
- f1d92061 - Now clearing batch after applying suggestion batch.
- dd368eb7 - Refactoring methods
- d41b24eb - added capybara tests
- 14354ce0 - updated temporary variable names
- 85cf880e - added a couple of tests for the new actions. still need to the the submit batch action.
- 0ef2e7ca - Passiing first test for submitSuggestionBatch action.
- 9ba6b512 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- ce191f1e - Updates to tests.
- 14a35b1b - Updates to vue components and tests.
- 74e698b5 - code review revisions
- 01efbd51 - New api endpoint for applying suggestion batches
- 250acc47 - Updates to docs and error messages.
- 290c74a2 - Was not hiding loading spinner after failing to apply batch.
- 2e3241da - Updates to docs and default commit message.
- 641b6f31 - Refactoring apply_service.rb
- f9818ac2 - Additional refactoring
- 15d6dd5c - Updates after latest review
- d4124696 - Changes following additional review
- 18131052 - Missing green text within badge pill.
- c2913127 - Added check for empty batches in api/suggestions.rb
- d0266829 - Fix for weird transition visuals between "applying" and "applied" states.
- 9a05e8b2 - Split apply_service responsibilities into different classes.
- 6eb603a0 - Resotres last commit id parameter for multi-service actions.
- ffc7c133 - code review changes
- 1fee11b8 - Code Review Changes
Toggle commit list-
955f9deb...b6b8ee4e - 1216 commits from branch
added 122 commits
-
1fee11b8...7596dd3d - 91 commits from branch
gitlab-org:master
- e78ee748 - Can apply suggestions in batches.
- fd270f71 - Updates to tests in apply_service_spec
- 6d3137a2 - Changes to variable names.
- 9321c381 - can add and remove suggestions from a batch
- b4012444 - Can apply multiple suggestions through the front end.
- abc93ccb - Can get loading spinner when applying batch suggestions.
- 40836728 - Now clearing batch after applying suggestion batch.
- ce1a1bf7 - Refactoring methods
- 5381567e - added capybara tests
- 7fcd4c25 - updated temporary variable names
- 568ed712 - added a couple of tests for the new actions. still need to the the submit batch action.
- f0203b12 - Passiing first test for submitSuggestionBatch action.
- 2cf53ed3 - Updated logic in submitSuggestionsBatch action. updated note actions test.
- af5e6afa - Updates to tests.
- 382911bc - Updates to vue components and tests.
- b00c3b96 - code review revisions
- 5b60d49d - New api endpoint for applying suggestion batches
- ae389c93 - Updates to docs and error messages.
- eeced0c9 - Was not hiding loading spinner after failing to apply batch.
- e0453e72 - Updates to docs and default commit message.
- c4efce82 - Refactoring apply_service.rb
- 1944b2e1 - Additional refactoring
- df09cdc3 - Updates after latest review
- ad277d34 - Changes following additional review
- a7f96848 - Missing green text within badge pill.
- 6747b353 - Added check for empty batches in api/suggestions.rb
- 85fa2d3f - Fix for weird transition visuals between "applying" and "applied" states.
- 01250873 - Split apply_service responsibilities into different classes.
- dc086b92 - Resotres last commit id parameter for multi-service actions.
- 714807c9 - code review changes
- 05e65ec4 - Code Review Changes
Toggle commit list-
1fee11b8...7596dd3d - 91 commits from branch
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
Thanks @jessehall3, I think we're getting there, I've left some thoughts on the spec you were refering to for batch-applying suggestions across branches. I'm also still worried about the
last_commit_id
thing. Let me know what you think.
added 849 commits
-
1aba3714...781363dc - 818 commits from branch
gitlab-org:master
- c80b9002 - Can apply suggestions in batches.
- 1d040f7d - Updates to tests in apply_service_spec
- cf1a9780 - Changes to variable names.
- 28f9154f - can add and remove suggestions from a batch
- f867fb1d - Can apply multiple suggestions through the front end.
- fb6cb33d - Can get loading spinner when applying batch suggestions.
- d07d53f4 - Now clearing batch after applying suggestion batch.
- fdfb4d18 - Refactoring methods
- f0844aea - added capybara tests
- 7deadaf1 - updated temporary variable names
- 6c908e5d - added a couple of tests for the new actions. still need to the the submit batch action.
- fc10eb43 - Passiing first test for submitSuggestionBatch action.
- e7affdaf - Updated logic in submitSuggestionsBatch action. updated note actions test.
- ccf07be1 - Updates to tests.
- d0518272 - Updates to vue components and tests.
- aec661d4 - code review revisions
- 96dbe5a3 - New api endpoint for applying suggestion batches
- e9a8181a - Updates to docs and error messages.
- b9f9c39d - Was not hiding loading spinner after failing to apply batch.
- 19ada038 - Updates to docs and default commit message.
- e82a48f1 - Refactoring apply_service.rb
- 918c1de6 - Additional refactoring
- 8d9a7998 - Updates after latest review
- 25df175d - Missing green text within badge pill.
- 6291f066 - Added check for empty batches in api/suggestions.rb
- d816b543 - Fix for weird transition visuals between "applying" and "applied" states.
- 58d515dd - Split apply_service responsibilities into different classes.
- e45c6ced - Resotres last commit id parameter for multi-service actions.
- 27df8c0c - code review changes
- d7358dfc - Code Review Changes
- cd7b9bab - Code review changes.
Toggle commit list-
1aba3714...781363dc - 818 commits from branch
- Resolved by Daniel Gruesso
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Bob Van Landuyt
- Resolved by Mark Florian
@jessehall3 Thank you very much for your persistence in this feature! I'm down to nitpicks now. But let's get some more input from frontend though, I saw it doesn't have approval yet. @jboyson I saw you already started this, do you want to take another look? Or is this ready for maintainer review?
assigned to @jboyson
added 1085 commits
-
b788c282...6c39d8d1 - 1084 commits from branch
gitlab-org:master
- f6ce76c8 - Feature for #25486 (closed), users can apply multiple suggestions at once.
-
b788c282...6c39d8d1 - 1084 commits from branch
added 17 commits
-
f6ce76c8...6f114149 - 16 commits from branch
gitlab-org:master
- 66b6fd70 - Feature for #25486 (closed), users can apply multiple suggestions at once.
-
f6ce76c8...6f114149 - 16 commits from branch
- Resolved by Mark Florian
Thanks @jessehall3, from a backend perspective, I have nothing to add anymore. But let me know if there's things you want me to double check after frontend review. Thank you very much!
I'll gladly give it another run-through after that before merging
.Edited by Bob Van Landuyt
- Resolved by Justin Boyson
@jboyson friendly poke about leaving a review
- Resolved by Justin Boyson
- Resolved by Mark Florian
- Resolved by Justin Boyson
- Resolved by Justin Boyson
- Resolved by Mark Florian
@jessehall3 looking great! Just a few tiny suggestions and I think we can finally get this over to a maintainer
added 882 commits
-
66b6fd70...2ca22554 - 880 commits from branch
gitlab-org:master
- ed2bf1c5 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- f4691403 - Now using optional chaining.
-
66b6fd70...2ca22554 - 880 commits from branch
added 1473 commits
-
f4691403...d58353db - 1471 commits from branch
gitlab-org:master
- 57797a3c - Feature for #25486 (closed), users can apply multiple suggestions at once.
- a4c6b6c0 - Now using optional chaining.
-
f4691403...d58353db - 1471 commits from branch
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Daniel Gruesso
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
@jessehall3 This is looking great on the whole after my first review pass! I've left various comments and suggestions, so for now it's back to you
Aside: I've left various comments using the Conventional Comments style. If you have any feedback on whether this was useful to you, that'd be great!
@markrian
First, I do appreciate the Conventional Comments style used here, particularly the emphasis on what is, and what is not, blocking. I'm going to be trying this style myself.That said, I responded to all your comments. Please let me your thoughts, thank you.
mentioned in issue #216489
- Resolved by Mark Florian
- Resolved by Daniel Gruesso
Thanks @jessehall3! I've left a few more comments, but on the whole this is looking great. The main frontend blockers are the UX questions for @pedroms.
Back to you for now!
Edited by Mark Florian
added 798 commits
-
1053342c...72788117 - 794 commits from branch
gitlab-org:master
- 54e4e3d2 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- 83ddd7dd - Now using optional chaining.
- 716c9d51 - Code review changes
- 9cd8de22 - Applied patch for test and removed action.
Toggle commit list-
1053342c...72788117 - 794 commits from branch
unassigned @pedroms
added 886 commits
-
9cd8de22...1f35f62e - 881 commits from branch
gitlab-org:master
- dee2e9ee - Feature for #25486 (closed), users can apply multiple suggestions at once.
- 39a993c2 - Now using optional chaining.
- f489d0d7 - Code review changes
- a971b008 - Applied patch for test and removed action.
- 146fcd82 - Suggestion buttons with affirmitive actions now on the right hand side.
Toggle commit list-
9cd8de22...1f35f62e - 881 commits from branch
- Resolved by Daniel Gruesso
mentioned in epic &3280 (closed)
Thanks @jessehall3! I've left just one more comment; otherwise, I think the frontend is looking good!
I want to give this another pass later this week. If I find nothing else, I'll approve.
In any case, I think it's time to bring @marcia back in to help complete the missing documentation. After that,
@reprazent
said he'd like to take another look just before merging.Finally, I don't think this will make %13.0, and I don't think we should rush to try to fit it in, so the documentation/images will need to be updated/renamed to reflect that.
Nearly there!
assigned to @marcia
@markrian Thanks for these updates and all your help. I'll look at this latest comment and wait to hear more later on. No problem with the docs/images, I will update them to match 13.1. Take care.
unassigned @marcia
changed milestone to %13.1
added 812 commits
-
146fcd82...99d21e45 - 811 commits from branch
gitlab-org:master
- d827c41d - Feature for #25486 (closed), users can apply multiple suggestions at once.
-
146fcd82...99d21e45 - 811 commits from branch
- Resolved by Daniel Gruesso
Thanks @jessehall3!
I've done another pass, and have added one more suggestion. Once you address that, I'll be happy to approve the frontend portion of this MR! In fact, I'll pre-emptively do so, since the suggestion is so trivial.
Fantastic work, and I can't wait to see this released!
unassigned @markrian
added 1 commit
- 7a86f27f - Feature for #25486 (closed), users can apply multiple suggestions at once.
added 138 commits
-
7a86f27f...2d3117cf - 137 commits from branch
gitlab-org:master
- 3323be20 - Feature for #25486 (closed), users can apply multiple suggestions at once.
-
7a86f27f...2d3117cf - 137 commits from branch
unassigned @pedroms
- Resolved by Pedro Moreira da Silva
added 570 commits
-
3323be20...da276110 - 569 commits from branch
gitlab-org:master
- 99bad57d - Feature for #25486 (closed), users can apply multiple suggestions at once.
-
3323be20...da276110 - 569 commits from branch
added 1 commit
- c3de448b - Feature for #25486 (closed), users can apply multiple suggestions at once.
added 16 commits
-
5a1d47b2...b969416f - 13 commits from branch
gitlab-org:master
- 519e5f72 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- 2651d989 - Now pluralizing applying suggestions message.
- d41f4d5e - Now reseting applying batch state after applying single suggestion or batch.
Toggle commit list-
5a1d47b2...b969416f - 13 commits from branch
added 9 commits
-
d41f4d5e...f9ac8102 - 6 commits from branch
gitlab-org:master
- 04a5b9b5 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- aa582d5d - Now pluralizing applying suggestions message.
- d8a9947f - Now reseting applying batch state after applying single suggestion or batch.
Toggle commit list-
d41f4d5e...f9ac8102 - 6 commits from branch
- Resolved by Mark Florian
- Resolved by Daniel Gruesso
@markrian I was wondering if you could have a look at the last two commits. In the first one, as @pedroms pointed out, I was missing pluralization in one spot. In making this change, it seemed like a good idea to update one of the actions, that is the second commit. I put details in my latest comment.
added 935 commits
-
d8a9947f...a91f7e32 - 932 commits from branch
gitlab-org:master
- 95ecb020 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- 6d01fa08 - Now pluralizing applying suggestions message.
- e5cbb9b8 - Now reseting applying batch state after applying single suggestion or batch.
Toggle commit list-
d8a9947f...a91f7e32 - 932 commits from branch
- Resolved by Marcia Ramos
Now that's it's received frontend, backend, and UX reviews, I wanted to ask if we could complete the work on the docs related to this MR.
Thanks and take care.
assigned to @marcia
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
- Resolved by Daniel Gruesso
unassigned @marcia
added 1232 commits
-
e5cbb9b8...68330fb8 - 1228 commits from branch
gitlab-org:master
- 159104ff - Feature for #25486 (closed), users can apply multiple suggestions at once.
- 400e430d - Now pluralizing applying suggestions message.
- 7e86254f - Now reseting applying batch state after applying single suggestion or batch.
- 2d13e4b0 - doc updates
Toggle commit list-
e5cbb9b8...68330fb8 - 1228 commits from branch
mentioned in issue #30707 (closed)
assigned to @marcia
- Resolved by Daniel Gruesso
unassigned @marcia
added 1 commit
- cd152239 - Apply suggestion to doc/user/discussions/index.md
- Resolved by Bob Van Landuyt
@jessehall3 can you please resolve conflicts? Looks close. Please confirm we can remove WIP status now. Thanks.
Edited by Daniel Gruesso
added 1035 commits
-
cd152239...89b3e1dc - 1030 commits from branch
gitlab-org:master
- 445649ed - Feature for #25486 (closed), users can apply multiple suggestions at once.
- c3cfe5fe - Now pluralizing applying suggestions message.
- 38d3c91a - Now reseting applying batch state after applying single suggestion or batch.
- 676e6e06 - doc updates
- 38a85ef6 - Apply suggestion to doc/user/discussions/index.md
Toggle commit list-
cd152239...89b3e1dc - 1030 commits from branch
added 10 commits
-
38a85ef6...92d173c7 - 5 commits from branch
gitlab-org:master
- ca7ce2e1 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- a1af19f4 - Now pluralizing applying suggestions message.
- 0be22a60 - Now reseting applying batch state after applying single suggestion or batch.
- 40f98041 - doc updates
- 97dd3eb5 - Apply suggestion to doc/user/discussions/index.md
Toggle commit list-
38a85ef6...92d173c7 - 5 commits from branch
added 15 commits
-
97dd3eb5...0813a73d - 10 commits from branch
gitlab-org:master
- 92890707 - Feature for #25486 (closed), users can apply multiple suggestions at once.
- fff9df16 - Now pluralizing applying suggestions message.
- 4f0d570d - Now reseting applying batch state after applying single suggestion or batch.
- 782e5afb - doc updates
- 402ed928 - Apply suggestion to doc/user/discussions/index.md
Toggle commit list-
97dd3eb5...0813a73d - 10 commits from branch
assigned to @reprazent
enabled an automatic merge when the pipeline for cac9ad10 succeeds
Gave this another run, works like a charm! Thanks @jessehall3! I've triggered a new pipeline and cancelled the stuck one, I hope that does it
.mentioned in commit a7fe3895
mentioned in issue gitlab-com/gl-infra/scalability#408
@reprazent, a big thanks for literally getting this unstuck. ;)
More thanks to everyone who helped: @dskim_gitlab @marcia @markrian @pedroms @jboyson @jramsay @danielgruesso @m_gill @oswaldo @axil @rpaik @andr3
@jessehall3 thank you so much for all the hard work here, this is a great contribution on all levels! And thank you for bearing with us
added workflowstaging label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
mentioned in merge request !33357 (merged)
Wow, this feature is AMAZINGGGGGGGGGGGG. Thank you so much for implementing this @jessehall3
mentioned in merge request gitlab-com/www-gitlab-com!52665 (closed)
mentioned in issue #221222
mentioned in epic gitlab-com&443 (closed)
WOW! This is great work!! Thanks @jessehall3
Great work @jessehall3 - this will improve my day-to-day workflow significantly :)
mentioned in merge request !34782 (merged)
mentioned in merge request gitlab-com/www-gitlab-com!53361 (merged)
mentioned in issue gitlab-com/www-gitlab-com#7207 (closed)
Hi! Thanks for your hard work on this great feature. We wished it was there since the suggestions was released, because tons of commits looks ugly. I updated our gitlab free instance to 13.1.0 (d62ee60b669) yesterday, and I can't make this feature work. I even enabled the
:batched_suggestions
feature toggle discussed here (by the way, should I do it, or this document is outdated?)I also started
gitlab-ctl reconfigure
, but it still does not help.The button does not appear:
What am I doing wrong?
Edited by Himura KazutoSo it seems like I'm not the only one. I thought I did something wrong... also toggled the feature flag. Using the current Gitlab Docker, but the add button does not appear.
Edit: Ofc thanks so much for this very needed feature!
Edited by Florian Pirmin GroetznerLooks like the feature flag is actually called
batch_suggestions
I'll open an MR to update the documentation right now
Oh! Looks like it's already updated just a few hours ago.
It works! Thanks a lot!!! I removed the bad feature-flag using
Feature.remove(:batched_suggestions)
Edited by Himura Kazuto@himura @Dlotan My apologies for missing these messages, I had filtered out these notifications at the time and am just now seeing them. Regardless, I'm glad you were able to get this resolved rather quickly.
Edited by Jesse Hall
mentioned in merge request gitlab-com/www-gitlab-com!56398 (merged)
mentioned in issue #262637 (closed)
mentioned in issue gitlab-com/www-gitlab-com#6581 (closed)
mentioned in issue gitlab-com/www-gitlab-com#9384 (closed)
mentioned in issue gitlab-com/www-gitlab-com#9525 (closed)
mentioned in issue gitlab-com/www-gitlab-com#9629 (closed)
mentioned in issue gitlab-com/www-gitlab-com#9664 (closed)
mentioned in issue #276943 (closed)
added Category:Code Review Workflow groupcode review labels and removed groupsource code label
mentioned in merge request gitlab-com/www-gitlab-com!73161 (merged)
mentioned in issue ux-research#1473 (closed)
added typemaintenance label