Conversation
…in backend and mobile UI
… status, and implement deal filtering/sorting logic
…ervice and AuthService
… indicators, and assignee avatars, while adding resolution SLA tracking and backend filtering support.
…s, and update task model fields
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| return fail(400, { | ||
| errors, | ||
| data: { phone } | ||
| data: { name, phone } |
There was a problem hiding this comment.
'object short notation' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
Too many errors. (71% scanned).
| const org = locals.org; | ||
| const formData = await request.formData(); | ||
| const phone = formData.get('phone')?.toString(); | ||
| const name = formData.get('name')?.toString() ?? ''; |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
Expected ':' and instead saw 'toString'.
Expected an assignment or function call and instead saw an expression.
Expected an identifier and instead saw '.'.
Expected an operator and instead saw '?'.
Missing semicolon.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (70)
📝 WalkthroughWalkthroughBackend adds shared list filtering, fixes tag filters, simplifies comment creation, and introduces a persisted user name. Frontend profile editing now updates name. Mobile integrates token refresh and Crashlytics, expands core models, and refactors deals, leads, tasks, tickets, and common UI with new providers, screens, and interactions. ChangesBackend: list filters, comments path, and user name field
Frontend (web): Profile name edit wiring
Mobile platform: Crashlytics + token refresh/auth integration
Mobile shared models and lookups
Mobile deals: providers, screens, widgets, tests
Mobile leads: provider, list/detail, card
Mobile tasks: provider, list/detail/form, row widget
Mobile tickets: provider and list UI
Mobile common UI/routing and settings/profile
Sequence Diagram(s)sequenceDiagram
participant Mobile UI
participant ApiService
participant AuthService
participant Backend
Mobile UI->>ApiService: GET (requiresAuth)
ApiService->>Backend: Request with access token
Backend-->>ApiService: 401 Unauthorized
ApiService->>AuthService: refreshAccessToken()
AuthService-->>ApiService: true (refreshed)
ApiService->>Backend: Retry original request
Backend-->>ApiService: 200 OK (data)
ApiService-->>Mobile UI: Response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~180 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
A grab-bag "Dev" PR that adds Firebase Crashlytics dependencies, refactors the kanban column (per-currency totals, multi-select hooks, theming via DealStage.color), adds a closed-lost banner and broader stage-tap behavior to StageStepper, introduces a widget test suite for DealsListScreen, and modifies app_shell.dart (the diff for which was truncated and could not be fully reviewed).
Changes:
- Add
firebase_core/firebase_crashlytics(Android-only intent) andflutter_driverdev dependency. - Refactor
KanbanColumnto derive stage color fromDealStage.color, expose selection / long-press hooks, and replace the singlecurrencySymboltotal with a per-currency "dominant bucket + mixed" header. - Add closed-lost banner and make all non-current cells tappable in
StageStepper; add widget tests covering list view, drag-to-stage, and pagination.
Reviewed changes
Copilot reviewed 65 out of 71 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| mobile/pubspec.yaml | Adds Firebase Crashlytics + flutter_driver dependencies. |
| mobile/pubspec.lock | Resolved versions for the new dependencies and their transitives. |
| mobile/lib/widgets/misc/stage_stepper.dart | Adds closed-lost banner and broadens which stage cells are tappable. |
| mobile/lib/widgets/misc/kanban_column.dart | Per-currency header total, selection state, long-press hooks, theme-driven stage color. |
| mobile/lib/widgets/common/app_shell.dart | Diff truncated — not fully reviewed. |
| mobile/test/screens/deals/deals_list_screen_test.dart | New widget tests for list view, drag-to-stage, and pagination behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+113
to
+115
| onDragStarted: selected | ||
| ? null | ||
| : () => onDealLongPress?.call(deal), |
Comment on lines
+32
to
47
| ({Currency currency, double total, bool mixed}) _dominantBucket() { | ||
| if (deals.isEmpty) { | ||
| return (currency: Currency.usd, total: 0, mixed: false); | ||
| } | ||
| final Map<Currency, double> totals = {}; | ||
| for (final deal in deals) { | ||
| totals[deal.currency] = (totals[deal.currency] ?? 0) + deal.value; | ||
| } | ||
| final entries = totals.entries.toList() | ||
| ..sort((a, b) => b.value.compareTo(a.value)); | ||
| return ( | ||
| currency: entries.first.key, | ||
| total: entries.first.value, | ||
| mixed: entries.length > 1, | ||
| ); | ||
| } |
Comment on lines
+89
to
+92
| Future<void> _switchToListView(WidgetTester tester) async { | ||
| await tester.tap(find.byType(IconButton).at(1)); | ||
| await tester.pumpAndSettle(); | ||
| } |
Comment on lines
+22
to
+24
| # Firebase / Crashlytics (Android only — no iOS GoogleService-Info.plist yet) | ||
| firebase_core: ^3.10.0 | ||
| firebase_crashlytics: ^4.2.0 |
Comment on lines
+94
to
+111
| Future<void> _dragDealToStage( | ||
| WidgetTester tester, | ||
| String dealTitle, | ||
| DealStage targetStage, | ||
| ) async { | ||
| final dealCenter = tester.getCenter(find.text(dealTitle)); | ||
| final targetRect = tester.getRect( | ||
| find.byType(DragTarget<Deal>).at(targetStage.stageIndex), | ||
| ); | ||
| final targetPoint = Offset(targetRect.left + 48, targetRect.top + 120); | ||
|
|
||
| final gesture = await tester.startGesture(dealCenter); | ||
| await tester.pump(kLongPressTimeout + const Duration(milliseconds: 100)); | ||
| await gesture.moveTo(targetPoint); | ||
| await tester.pump(); | ||
| await gesture.up(); | ||
| await tester.pumpAndSettle(); | ||
| } |
Comment on lines
+160
to
+166
| // Any non-current cell is tappable when onStageChange is wired up. | ||
| // Backward moves correct mistakes; forward moves advance the deal; | ||
| // any tap on a lost deal reopens it into the chosen stage. | ||
| final isTappable = !isCurrent && onStageChange != null; | ||
|
|
||
| return GestureDetector( | ||
| onTap: isFuture && onStageChange != null | ||
| ? () => onStageChange!(stage.stage) | ||
| : null, | ||
| onTap: isTappable ? () => onStageChange!(stage.stage) : null, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Improvements