Skip to content

Commit dc0a300

Browse files
authored
feat(merge_requests): select and confirm suggestion batches (#593)
* feat(merge_requests): select and confirm suggestion batches Review every selected patch and freshly inspect all selected discussions before one batch write; require complete read-only recovery after uncertain confirmation while preserving drafts and session isolation. Signed-off-by: sjungwon03 <sjungwon03@gmail.com> * docs(parity): link batch suggestion UI PR Link the complete batch UI slice and retain multiline review as the remaining parity follow-up. Signed-off-by: sjungwon03 <sjungwon03@gmail.com> --------- Signed-off-by: sjungwon03 <sjungwon03@gmail.com>
1 parent 602fa76 commit dc0a300

27 files changed

Lines changed: 2463 additions & 19 deletions

‎apps/labfox/lib/features/comments/data/comments_repository.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@ class CommentsRepository {
3535
String? commitMessage,
3636
}) => _client.suggestions.apply(suggestionId, commitMessage: commitMessage);
3737

38+
Future<List<Suggestion>> applySuggestions({
39+
required List<int> suggestionIds,
40+
String? commitMessage,
41+
}) => _client.suggestions.applyBatch(
42+
suggestionIds,
43+
commitMessage: commitMessage,
44+
);
45+
3846
Future<Note> replyToDiscussion({
3947
required int projectId,
4048
required int iid,

‎apps/labfox/lib/features/comments/data/suggestion_application.dart‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import 'package:gitlab_models/gitlab_models.dart';
22

3+
typedef SuggestionTarget = ({
4+
String discussionId,
5+
Note note,
6+
Suggestion suggestion,
7+
});
8+
39
/// Patch eligibility is not a permission check: GitLab enforces write access.
410
bool canApplySuggestion(Note note, Suggestion suggestion) =>
511
!note.isSystem &&
@@ -53,3 +59,37 @@ bool confirmsAppliedSuggestion(Suggestion returned, Suggestion expected) =>
5359
(returned.fromContent == null ||
5460
returned.fromContent == expected.fromContent) &&
5561
(returned.toContent == null || returned.toContent == expected.toContent);
62+
63+
List<SuggestionTarget> applicableSuggestionTargets(List<Discussion> groups) {
64+
final counts = <int, int>{};
65+
for (final group in groups) {
66+
for (final note in group.notes) {
67+
for (final suggestion in note.suggestions ?? <Suggestion>[]) {
68+
counts.update(suggestion.id, (v) => v + 1, ifAbsent: () => 1);
69+
}
70+
}
71+
}
72+
return List<SuggestionTarget>.unmodifiable([
73+
for (final group in groups)
74+
for (final note in group.notes)
75+
for (final suggestion in note.suggestions ?? <Suggestion>[])
76+
if (counts[suggestion.id] == 1 &&
77+
canApplySuggestion(note, suggestion))
78+
(discussionId: group.id, note: note, suggestion: suggestion),
79+
]);
80+
}
81+
82+
bool containsApplicableSuggestionTargets(
83+
List<Discussion> groups,
84+
List<SuggestionTarget> targets,
85+
) =>
86+
targets.length >= 2 &&
87+
targets.map((t) => t.suggestion.id).toSet().length == targets.length &&
88+
targets.every(
89+
(t) => containsApplicableSuggestion(
90+
groups,
91+
discussionId: t.discussionId,
92+
note: t.note,
93+
suggestion: t.suggestion,
94+
),
95+
);

‎apps/labfox/lib/features/merge_requests/presentation/controllers/mr_discussions_controller.dart‎

Lines changed: 111 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,96 @@ class MrDiscussionsController
214214
}
215215
}
216216

217+
bool _currentSuggestions(List<SuggestionTarget> targets) =>
218+
!state.isLoading &&
219+
!state.hasError &&
220+
state.hasValue &&
221+
containsApplicableSuggestionTargets(state.value!.items, targets);
222+
223+
/// Confirms all selected patches before one batch; never applies a subset.
224+
Future<bool> applySuggestions(
225+
List<SuggestionTarget> selection, {
226+
String? commitMessage,
227+
bool Function()? isCurrent,
228+
}) async {
229+
final targets = List<SuggestionTarget>.unmodifiable(selection);
230+
if (_posting ||
231+
_loadingMore ||
232+
!_currentSuggestions(targets) ||
233+
isCurrent?.call() == false) {
234+
return false;
235+
}
236+
final generation = _generation;
237+
_posting = true;
238+
bool current() => generation == _generation && isCurrent?.call() != false;
239+
try {
240+
final repo = await _repository();
241+
if (!current()) return false;
242+
if (repo == null) {
243+
throw const GitLabAuthException('No authenticated account.');
244+
}
245+
if (!_currentSuggestions(targets)) return false;
246+
final fresh = await _readDiscussions(
247+
repo,
248+
targets.map((t) => t.discussionId).toSet().toList(),
249+
current,
250+
);
251+
if (!current() || fresh == null) return false;
252+
if (!_currentSuggestions(targets) ||
253+
!containsApplicableSuggestionTargets(fresh, targets)) {
254+
throw const GitLabConflictException(
255+
'Suggestions changed. Reload before applying.',
256+
);
257+
}
258+
final returned = await repo.applySuggestions(
259+
suggestionIds: targets.map((t) => t.suggestion.id).toList(),
260+
commitMessage: commitMessage,
261+
);
262+
if (!current()) return false;
263+
final byId = {for (final s in returned) s.id: s};
264+
if (returned.length != targets.length ||
265+
byId.length != targets.length ||
266+
!targets.every(
267+
(t) =>
268+
byId[t.suggestion.id] != null &&
269+
confirmsAppliedSuggestion(byId[t.suggestion.id]!, t.suggestion),
270+
)) {
271+
throw const GitLabServerException(
272+
'Unconfirmed suggestion batch application.',
273+
);
274+
}
275+
ref.invalidate(mergeRequestControllerProvider(arg));
276+
ref.invalidate(mrReviewSnapshotControllerProvider(arg));
277+
ref.invalidateSelf();
278+
return true;
279+
} catch (_) {
280+
if (!current()) return false;
281+
rethrow;
282+
} finally {
283+
if (generation == _generation) _posting = false;
284+
}
285+
}
286+
217287
/// Explicit read-only recovery never retries an application automatically.
218288
Future<Discussion?> inspectDiscussion(
219289
String discussionId, {
220290
bool Function()? isCurrent,
291+
}) async =>
292+
(await inspectDiscussions([discussionId], isCurrent: isCurrent))?.single;
293+
294+
/// Stages every selected thread before replacing any loaded cache entry.
295+
Future<List<Discussion>?> inspectDiscussions(
296+
List<String> discussionIds, {
297+
bool Function()? isCurrent,
221298
}) async {
222-
if (_posting || _loadingMore || isCurrent?.call() == false) return null;
299+
final ids = List<String>.unmodifiable(discussionIds);
300+
if (_posting ||
301+
_loadingMore ||
302+
isCurrent?.call() == false ||
303+
ids.isEmpty ||
304+
ids.toSet().length != ids.length) {
305+
return null;
306+
}
223307
final generation = _generation;
224308
_posting = true;
225309
bool current() => generation == _generation && isCurrent?.call() != false;
@@ -229,21 +313,15 @@ class MrDiscussionsController
229313
if (repo == null) {
230314
throw const GitLabAuthException('No authenticated account.');
231315
}
232-
final fresh = await repo.discussion(
233-
projectId: arg.projectId,
234-
iid: arg.iid,
235-
discussionId: discussionId,
236-
);
237-
if (!current()) return null;
238-
if (fresh.id != discussionId) {
239-
throw const GitLabServerException('Invalid discussion response.');
240-
}
316+
final fresh = await _readDiscussions(repo, ids, current);
317+
if (!current() || fresh == null) return null;
241318
final page = state.valueOrNull;
242319
if (page != null && !state.isLoading && !state.hasError) {
320+
final byId = {for (final g in fresh) g.id: g};
243321
state = AsyncData(
244322
Paginated(
245323
items: List<Discussion>.unmodifiable(
246-
page.items.map((g) => g.id == discussionId ? fresh : g),
324+
page.items.map((g) => byId[g.id] ?? g),
247325
),
248326
nextPage: page.nextPage,
249327
total: page.total,
@@ -260,6 +338,28 @@ class MrDiscussionsController
260338
}
261339
}
262340

341+
Future<List<Discussion>?> _readDiscussions(
342+
CommentsRepository repo,
343+
List<String> ids,
344+
bool Function() isCurrent,
345+
) async {
346+
final result = <Discussion>[];
347+
for (final id in ids) {
348+
if (!isCurrent()) return null;
349+
final fresh = await repo.discussion(
350+
projectId: arg.projectId,
351+
iid: arg.iid,
352+
discussionId: id,
353+
);
354+
if (!isCurrent()) return null;
355+
if (fresh.id != id) {
356+
throw const GitLabServerException('Invalid discussion response.');
357+
}
358+
result.add(fresh);
359+
}
360+
return List<Discussion>.unmodifiable(result);
361+
}
362+
263363
Future<bool> _post(
264364
String body, {
265365
String? discussionId,

‎apps/labfox/lib/features/merge_requests/presentation/widgets/mr_discussion_thread.dart‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import '../controllers/merge_requests_controllers.dart';
1414
import '../controllers/mr_discussions_controller.dart';
1515
import 'mr_discussion_position_context.dart';
1616
import 'mr_suggestion_apply_dialog.dart';
17+
import 'mr_suggestion_batch_apply_dialog.dart';
1718
import 'mr_suggestion_preview.dart';
1819

1920
/// Grouped MR replies with explicit pagination and the shared note composer.
@@ -202,6 +203,49 @@ class _ConversationState extends ConsumerState<_Conversation> {
202203
}
203204
}
204205

206+
Future<void> _applyBatch(List<SuggestionTarget> targets) async {
207+
if (_applicationId != null ||
208+
_topPosting ||
209+
_replyId != null ||
210+
_resolutionId != null ||
211+
widget.discussions.isLoading ||
212+
widget.discussions.hasError) {
213+
return;
214+
}
215+
final session = ref.read(commentsRepositoryProvider);
216+
final view = ValueNotifier(true);
217+
_applicationView = view;
218+
setState(() => _applicationId = 0);
219+
try {
220+
final applied = await showDialog<bool>(
221+
context: context,
222+
barrierDismissible: false,
223+
builder: (_) => MrSuggestionBatchApplyDialog(
224+
resource: widget.arg,
225+
targets: targets,
226+
session: session,
227+
isCurrent: () => mounted,
228+
viewActive: view,
229+
),
230+
);
231+
if (mounted &&
232+
applied == true &&
233+
ref.read(commentsRepositoryProvider) == session) {
234+
ScaffoldMessenger.of(context).showSnackBar(
235+
SnackBar(
236+
content: Text(
237+
AppLocalizations.of(context).mrSuggestionsApplySuccess,
238+
),
239+
),
240+
);
241+
}
242+
} finally {
243+
_applicationView = null;
244+
view.dispose();
245+
if (mounted) setState(() => _applicationId = null);
246+
}
247+
}
248+
205249
@override
206250
Widget build(BuildContext context) {
207251
final l10n = AppLocalizations.of(context);
@@ -248,12 +292,29 @@ class _ConversationState extends ConsumerState<_Conversation> {
248292
}
249293
}
250294
}
295+
final targets = applicableSuggestionTargets(page.items);
251296
final groups = page.items
252297
.where((group) => group.notes.any((note) => !note.isSystem))
253298
.toList();
254299
return Column(
255300
crossAxisAlignment: CrossAxisAlignment.stretch,
256301
children: [
302+
if (targets.length >= 2)
303+
Align(
304+
alignment: AlignmentDirectional.centerStart,
305+
child: TextButton(
306+
key: const ValueKey('mr-suggestions-batch-open'),
307+
onPressed:
308+
canPost &&
309+
_applicationId == null &&
310+
!_topPosting &&
311+
_replyId == null &&
312+
_resolutionId == null
313+
? () => _applyBatch(targets)
314+
: null,
315+
child: Text(l10n.mrSuggestionsBatchButton),
316+
),
317+
),
257318
if (groups.isEmpty)
258319
Text(
259320
page.hasMore ? l10n.mrDiscussionsPartial : l10n.commentsEmpty,

0 commit comments

Comments
 (0)