TF-4449 [Part 1] Add drive_attachment package data and domain layers#4587
Open
tddang-linagora wants to merge 1 commit into
Open
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new workplace Flutter package and integrates it into the workspace. The package provides JSON-serializable data models and enums, a Dio-based datasource and repository, domain entities and extensions (DriveDocument, WorkplaceIntent), parsing for intent messages into a sealed message hierarchy, UI/state classes, two use-case interactors (create intent, exchange token), and unit tests for the extension and message parsing. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4587. |
dab246
reviewed
Jun 5, 2026
dab246
reviewed
Jun 5, 2026
dab246
reviewed
Jun 5, 2026
dab246
reviewed
Jun 5, 2026
dab246
reviewed
Jun 5, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
dab246
reviewed
Jun 8, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
drive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dart (1)
44-46:⚠️ Potential issue | 🟠 MajorNarrow/document the debug-only HTTPS bypass for Drive intent URLs
DriveAttachmentDataSourceImpl.createIntent()currently accepts non-HTTPS intent URLs in debug builds (if (intentUrl.scheme != 'https' && !kDebugMode) { ... }), creating a behavior/security gap between debug and release.if (intentUrl.scheme != 'https' && !kDebugMode) { throw ArgumentError('Intent URL must use HTTPS, got: $href'); }The repo contains no non-test
http://usage indrive_attachment; this bypass should therefore be replaced with an explicit dev-only configuration (or constrained to safe loopback hosts likelocalhost/127.0.0.1) and documented for local development so it can’t be relied on as a general fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dart` around lines 44 - 46, The current HTTPS check in DriveAttachmentDataSourceImpl.createIntent() allows non-HTTPS schemes in debug builds via kDebugMode; replace this implicit debug bypass with an explicit dev-only configuration or allowlist (e.g., a boolean flag or env/config value like allowInsecureDevUrls) and/or restrict allowed non-HTTPS hosts to loopback addresses (localhost, 127.0.0.1) before permitting http://; update the intentUrl validation to consult that config/allowlist instead of kDebugMode and add a code comment and README note describing the dev-only behavior and how to enable it for local testing so the relaxed behavior cannot be relied upon outside explicit dev configuration.
🧹 Nitpick comments (1)
drive_attachment/test/drive_intent_message_test.dart (1)
107-111: ⚡ Quick winAssert the
typeproperty for consistency.The other unknown-type tests (lines 93-98 and 100-105) verify both the message class and the
typeproperty value. This test should also assert thetypeproperty to ensure the mismatched intentId is captured correctly.🧪 Suggested assertion to add
test('correct message type for different intentId is treated as unknown', () { final raw = encode({'type': 'intent-other-id:ready'}); final msg = DriveIntentMessage.parse(intentId, raw); expect(msg, isA<DriveIntentUnknownMessage>()); + expect((msg as DriveIntentUnknownMessage).type, 'intent-other-id:ready'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drive_attachment/test/drive_intent_message_test.dart` around lines 107 - 111, The test should also assert the message's type property for consistency: after parsing with DriveIntentMessage.parse and checking the result is a DriveIntentUnknownMessage, add an assertion that msg.type equals the original raw type string ('intent-other-id:ready') so the mismatched intentId case verifies both class and type; update the test block containing DriveIntentMessage.parse and the expect(msg, isA<DriveIntentUnknownMessage>()) to include this additional assertion on msg.type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@drive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dart`:
- Line 42: The code reads parsed.data.attributes.services.first into href
without guarding against an empty list; update the DriveAttachmentDatasourceImpl
logic to validate services is not null/empty before accessing .first (e.g.,
check parsed.data.attributes.services?.isNotEmpty), handle the empty case by
either throwing a clear error or using a safe fallback (choose and implement
consistent behavior), and ensure the value assigned to href is only taken after
this check so you avoid a StateError at runtime.
- Line 56: The code directly casts response.data['access_token'] to String which
will throw if the key is missing, null, or not a String; update the
token-returning method in DriveAttachmentDatasourceImpl to first retrieve
response.data['access_token'] into a local variable, validate that it is a
non-null String (and optionally non-empty), and only then return it, otherwise
throw a clear exception (e.g., FormatException or a custom error) or handle the
error path (null/default) so the method does not crash on unexpected API
responses.
In `@drive_attachment/lib/drive_attachment/domain/state/drive_intent_state.dart`:
- Around line 15-16: Change the constructor parameter type for
CreateDriveIntentFailure from dynamic to Object? to match project convention and
Effective Dart guidance: update the class CreateDriveIntentFailure(dynamic
exception) : super(exception: exception); to use Object? (i.e.,
CreateDriveIntentFailure(Object? exception) : super(exception: exception);) so
the exception is statically typed while still allowing null.
- Around line 32-33: The ExchangeDriveTokenFailure constructor currently types
its parameter as dynamic; change the parameter type to Object? to match project
convention and Effective Dart guidance by updating the
ExchangeDriveTokenFailure(dynamic exception) constructor to
ExchangeDriveTokenFailure(Object? exception) : super(exception: exception);
ensuring the class still extends FeatureFailure and passes the value to
super(exception: exception).
---
Duplicate comments:
In
`@drive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dart`:
- Around line 44-46: The current HTTPS check in
DriveAttachmentDataSourceImpl.createIntent() allows non-HTTPS schemes in debug
builds via kDebugMode; replace this implicit debug bypass with an explicit
dev-only configuration or allowlist (e.g., a boolean flag or env/config value
like allowInsecureDevUrls) and/or restrict allowed non-HTTPS hosts to loopback
addresses (localhost, 127.0.0.1) before permitting http://; update the intentUrl
validation to consult that config/allowlist instead of kDebugMode and add a code
comment and README note describing the dev-only behavior and how to enable it
for local testing so the relaxed behavior cannot be relied upon outside explicit
dev configuration.
---
Nitpick comments:
In `@drive_attachment/test/drive_intent_message_test.dart`:
- Around line 107-111: The test should also assert the message's type property
for consistency: after parsing with DriveIntentMessage.parse and checking the
result is a DriveIntentUnknownMessage, add an assertion that msg.type equals the
original raw type string ('intent-other-id:ready') so the mismatched intentId
case verifies both class and type; update the test block containing
DriveIntentMessage.parse and the expect(msg, isA<DriveIntentUnknownMessage>())
to include this additional assertion on msg.type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6e242206-09cc-48f5-805d-23cc41910fe9
📒 Files selected for processing (20)
drive_attachment/lib/drive_attachment/data/datasource/drive_attachment_datasource.dartdrive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dartdrive_attachment/lib/drive_attachment/data/model/drive_enums.dartdrive_attachment/lib/drive_attachment/data/model/drive_exchange_token_request.dartdrive_attachment/lib/drive_attachment/data/model/drive_request.dartdrive_attachment/lib/drive_attachment/data/model/drive_response.dartdrive_attachment/lib/drive_attachment/data/repository_impl/drive_attachment_repository_impl.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_attachment.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_document.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_document_extension.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_intent.dartdrive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dartdrive_attachment/lib/drive_attachment/domain/repository/drive_attachment_repository.dartdrive_attachment/lib/drive_attachment/domain/state/drive_intent_state.dartdrive_attachment/lib/drive_attachment/domain/usecase/create_drive_intent_interactor.dartdrive_attachment/lib/drive_attachment/domain/usecase/exchange_drive_token_interactor.dartdrive_attachment/pubspec.yamldrive_attachment/test/drive_document_extension_test.dartdrive_attachment/test/drive_intent_message_test.dartpubspec.yaml
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
dab246
previously approved these changes
Jun 8, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
zatteo
reviewed
Jun 10, 2026
| '$platformUrl/intents', | ||
| options: Options( | ||
| headers: {'Authorization': 'Bearer $accessToken'}, | ||
| extra: {'withCredentials': true}, |
Member
There was a problem hiding this comment.
I hope that at the end extra: {'withCredentials': true} won't be necessary anymore
| _ when type == 'intent-$intentId:ready' => const DriveIntentReadyMessage(), | ||
| _ when type == 'intent-$intentId:done' => DriveIntentDoneMessage.fromJson(map), | ||
| _ when type == 'intent-$intentId:error' => const DriveIntentErrorMessage(), | ||
| _ when type == 'intent-$intentId:cancel' => const DriveIntentCancelMessage(), |
Member
There was a problem hiding this comment.
Not an issue, but this intent protocol is not related to Drive. If one day we implement a "Visio" intent or "Chat" intent or whatever other apps it will use the same protocol.
hoangdat
reviewed
Jun 10, 2026
|
|
||
| const DriveAttachment({required this.document}); | ||
|
|
||
| String get linkedFileHeader => document.linkedFileHeader; |
| this.downloadLink, | ||
| }); | ||
|
|
||
| Uri? get attachmentUrl => sharingLink ?? downloadLink; |
| DriveAttachmentDataSourceImpl(this._dio); | ||
|
|
||
| @override | ||
| Future<DriveIntent> createIntent(Uri platformUrl, String accessToken) async { |
Member
There was a problem hiding this comment.
How about to keep platformURL in a riverpod provider?
Collaborator
Author
There was a problem hiding this comment.
will consider in part 2
tddang-linagora
force-pushed
the
feat/TF-4449-drive-attachment-data
branch
from
e8c6ec1 to
a58b282
Compare
tddang-linagora
had a problem deploying
to
PR-4587
GitHub Actions
Failure
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
drive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dart (3)
45-50: ⚡ Quick winType cast in
fromJsonis not defensive.Line 47 casts
json['document']toList?and line 48 casts each element toMap<String, dynamic>. If the server sendsdocumentas a non-List type (e.g.,{"document": "invalid"}), or if an array element is not a Map, the casts will throwTypeError.Verify whether the server contract guarantees
documentis alwaysnull, absent, or a list of maps. If not:🛡️ Defensive parsing option
factory DriveIntentDoneMessage.fromJson(Map<String, dynamic> json) => DriveIntentDoneMessage( - (json['document'] as List? ?? []) - .map((e) => DriveDocument.fromJson(e as Map<String, dynamic>)) + (json['document'] is List ? json['document'] as List : []) + .whereType<Map<String, dynamic>>() + .map((e) => DriveDocument.fromJson(e)) .toList(), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dart` around lines 45 - 50, DriveIntentDoneMessage.fromJson currently assumes json['document'] is a List and each element is a Map, which can throw TypeError on bad input; update the factory to defensively validate json['document'] (check that it is a List via `is List`), iterate only when it's a List, skip or filter elements that are not Map<String, dynamic>, and fall back to an empty list when the field is null/absent or invalid; use DriveDocument.fromJson only on validated Map entries so malformed payloads don't crash the parser.
5-15: ⚖️ Poor tradeoffException handling is inconsistent.
The function can throw
FormatExceptionfromjsonDecode(line 6) orTypeErrorfrom the Map cast (line 6), but neither is caught or wrapped inDriveIntentParseException. The exception class is defined (lines 53-59) but never thrown, making it dead code.Verify the intended error handling strategy:
- Should malformed JSON and type mismatches propagate as-is?
- Or should they be caught and wrapped in
DriveIntentParseException?If wrapping is intended:
DriveIntentMessage parseDriveIntentMessage(String intentId, String raw) { try { final map = jsonDecode(raw) as Map<String, dynamic>; // ... rest of logic } catch (e) { throw DriveIntentParseException(raw); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dart` around lines 5 - 15, The parseDriveIntentMessage function currently calls jsonDecode and casts to Map which can throw FormatException or TypeError but never wraps them; update parseDriveIntentMessage to catch parsing/type errors around the jsonDecode + cast and rethrow a DriveIntentParseException(raw) so malformed JSON or wrong types are encapsulated; keep the existing switch logic that returns DriveIntentReadyMessage, DriveIntentDoneMessage.fromJson, DriveIntentErrorMessage, DriveIntentCancelMessage, or DriveIntentUnknownMessage unchanged and only wrap exceptions from jsonDecode/cast into DriveIntentParseException.
53-59: ⚡ Quick win
DriveIntentParseExceptionis defined but never thrown.This exception class is never instantiated in the current code (see lines 5-15). Either:
- It's dead code that should be removed, or
parseDriveIntentMessageshould catch and wrap exceptions in it (see comment on lines 5-15)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dart` around lines 53 - 59, DriveIntentParseException is declared but never used; either delete the unused DriveIntentParseException class or update parseDriveIntentMessage to wrap parsing failures with it—inside parseDriveIntentMessage catch any parsing exceptions and rethrow a DriveIntentParseException containing the original raw input (optionally include the original error message/stack in the exception message) so the exception is actually emitted and can be handled by callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@drive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dart`:
- Around line 45-50: DriveIntentDoneMessage.fromJson currently assumes
json['document'] is a List and each element is a Map, which can throw TypeError
on bad input; update the factory to defensively validate json['document'] (check
that it is a List via `is List`), iterate only when it's a List, skip or filter
elements that are not Map<String, dynamic>, and fall back to an empty list when
the field is null/absent or invalid; use DriveDocument.fromJson only on
validated Map entries so malformed payloads don't crash the parser.
- Around line 5-15: The parseDriveIntentMessage function currently calls
jsonDecode and casts to Map which can throw FormatException or TypeError but
never wraps them; update parseDriveIntentMessage to catch parsing/type errors
around the jsonDecode + cast and rethrow a DriveIntentParseException(raw) so
malformed JSON or wrong types are encapsulated; keep the existing switch logic
that returns DriveIntentReadyMessage, DriveIntentDoneMessage.fromJson,
DriveIntentErrorMessage, DriveIntentCancelMessage, or DriveIntentUnknownMessage
unchanged and only wrap exceptions from jsonDecode/cast into
DriveIntentParseException.
- Around line 53-59: DriveIntentParseException is declared but never used;
either delete the unused DriveIntentParseException class or update
parseDriveIntentMessage to wrap parsing failures with it—inside
parseDriveIntentMessage catch any parsing exceptions and rethrow a
DriveIntentParseException containing the original raw input (optionally include
the original error message/stack in the exception message) so the exception is
actually emitted and can be handled by callers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5951f3a3-d7bf-4ce5-b5fb-53442306f4e7
📒 Files selected for processing (20)
drive_attachment/lib/drive_attachment/data/datasource/drive_attachment_datasource.dartdrive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dartdrive_attachment/lib/drive_attachment/data/model/drive_enums.dartdrive_attachment/lib/drive_attachment/data/model/drive_exchange_token_request.dartdrive_attachment/lib/drive_attachment/data/model/drive_request.dartdrive_attachment/lib/drive_attachment/data/model/drive_response.dartdrive_attachment/lib/drive_attachment/data/repository_impl/drive_attachment_repository_impl.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_attachment.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_document.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_document_extension.dartdrive_attachment/lib/drive_attachment/domain/entity/drive_intent.dartdrive_attachment/lib/drive_attachment/domain/message/drive_intent_message.dartdrive_attachment/lib/drive_attachment/domain/repository/drive_attachment_repository.dartdrive_attachment/lib/drive_attachment/domain/state/drive_intent_state.dartdrive_attachment/lib/drive_attachment/domain/usecase/create_drive_intent_interactor.dartdrive_attachment/lib/drive_attachment/domain/usecase/exchange_drive_token_interactor.dartdrive_attachment/pubspec.yamldrive_attachment/test/drive_document_extension_test.dartdrive_attachment/test/drive_intent_message_test.dartpubspec.yaml
✅ Files skipped from review due to trivial changes (1)
- drive_attachment/pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (16)
- drive_attachment/lib/drive_attachment/data/repository_impl/drive_attachment_repository_impl.dart
- drive_attachment/lib/drive_attachment/data/model/drive_exchange_token_request.dart
- drive_attachment/lib/drive_attachment/data/model/drive_response.dart
- drive_attachment/test/drive_document_extension_test.dart
- drive_attachment/lib/drive_attachment/domain/entity/drive_attachment.dart
- drive_attachment/lib/drive_attachment/domain/entity/drive_intent.dart
- pubspec.yaml
- drive_attachment/lib/drive_attachment/domain/state/drive_intent_state.dart
- drive_attachment/lib/drive_attachment/data/model/drive_enums.dart
- drive_attachment/lib/drive_attachment/data/model/drive_request.dart
- drive_attachment/lib/drive_attachment/domain/repository/drive_attachment_repository.dart
- drive_attachment/lib/drive_attachment/domain/entity/drive_document.dart
- drive_attachment/lib/drive_attachment/data/datasource/drive_attachment_datasource.dart
- drive_attachment/test/drive_intent_message_test.dart
- drive_attachment/lib/drive_attachment/domain/usecase/create_drive_intent_interactor.dart
- drive_attachment/lib/drive_attachment/data/datasource_impl/drive_attachment_datasource_impl.dart
hoangdat
reviewed
Jun 11, 2026
| @@ -0,0 +1,6 @@ | |||
| import 'package:drive_attachment/drive_attachment/domain/entity/drive_intent.dart'; | |||
|
|
|||
| abstract class DriveAttachmentDataSource { | |||
Member
There was a problem hiding this comment.
Suggested change
| abstract class DriveAttachmentDataSource { | |
| abstract class WorkplaceIntegrationDataSource { |
| part 'drive_exchange_token_request.g.dart'; | ||
|
|
||
| @JsonSerializable(fieldRename: FieldRename.snake, createFactory: false) | ||
| class DriveExchangeTokenRequest { |
Member
There was a problem hiding this comment.
Suggested change
| class DriveExchangeTokenRequest { | |
| class WrokplaceExchangeTokenRequest { |
| part 'drive_request.g.dart'; | ||
|
|
||
| @JsonSerializable(createFactory: false) | ||
| class DriveAttributesRequest { |
Member
There was a problem hiding this comment.
Suggested change
| class DriveAttributesRequest { | |
| class WorkplaceIntentAttributesRequest { |
| } | ||
|
|
||
| @JsonSerializable(createFactory: false) | ||
| class DriveDataRequest { |
Member
There was a problem hiding this comment.
Suggested change
| class DriveDataRequest { | |
| class WorkplaceIntentDataRequest { |
| } | ||
|
|
||
| @JsonSerializable(createFactory: false) | ||
| class DriveRequest { |
Member
There was a problem hiding this comment.
Suggested change
| class DriveRequest { | |
| class WorkplaceIntentRequest { |
| @@ -0,0 +1,11 @@ | |||
| import 'package:equatable/equatable.dart'; | |||
|
|
|||
| class DriveIntent with EquatableMixin { | |||
Member
There was a problem hiding this comment.
Suggested change
| class DriveIntent with EquatableMixin { | |
| class WorkplaceIntent with EquatableMixin { |
| ); | ||
| } | ||
|
|
||
| final class DriveIntentParseException implements Exception { |
|
|
||
| class CreatingDriveIntent extends LoadingState {} | ||
|
|
||
| class CreateDriveIntentSuccess extends UIState { |
| }); | ||
| }); | ||
|
|
||
| group('unknown type', () { |
Member
There was a problem hiding this comment.
should have variety cases:
- types
- multiple
: - multiple
intent - multiple
intent-$intentId - ... more and more case can make the parsing error
tddang-linagora
force-pushed
the
feat/TF-4449-drive-attachment-data
branch
from
a58b282 to
21253a7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
pubspec.yaml (1)
20-20: ⚡ Quick winFolder name should match package name for consistency.
The workspace entry
workplace(line 20) and dependency pathworkplace(line 78) reference a folder namedworkplace/, but the package itself is nameddrive_attachment(seeworkplace/pubspec.yamlline 1). This breaks the established naming pattern where all 11 existing workspace packages use matching folder and package names (e.g.,core/core,model/model,contact/contact, etc.).Consider renaming the
workplace/folder todrive_attachment/and updating these references accordingly. Since this is a new package with no existing consumers, the change is low-risk and would improve long-term maintainability.📁 Suggested folder rename and reference updates
- Rename folder:
workplace/→drive_attachment/- Update workspace entry:
- - workplace + - drive_attachment
- Update dependency path:
drive_attachment: - path: workplace + path: drive_attachmentAlso applies to: 77-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pubspec.yaml` at line 20, The workspace entry and dependency path currently reference the folder name "workplace" but the package inside is named "drive_attachment", so rename the folder from workplace/ to drive_attachment/ and update all references in pubspec.yaml: replace the workspace entry "workplace" and any dependency path "workplace" with "drive_attachment" (also update the dependency path under the package that declared the path), ensuring the workspace list and path dependencies match the package name "drive_attachment" used in the package's pubspec.yaml.workplace/lib/data/datasource_impl/workplace_datasource_impl.dart (2)
39-44: 💤 Low valueExtract
servicesto avoid redundant property access.The code accesses
parsed.data.attributes.servicestwice (lines 40 and 44). Extract it to a local variable for clarity.♻️ Proposed refactor
final intentId = parsed.data.id; final services = parsed.data.attributes.services; if (services.isEmpty) { throw StateError('Drive response contains no services'); } -final href = parsed.data.attributes.services.first.href; +final href = services.first.href; final intentUrl = Uri.parse(href);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart` around lines 39 - 44, The code reads parsed.data.attributes.services twice; extract parsed.data.attributes.services into a local variable (e.g., services) and use that variable for the emptiness check and to get services.first.href, keeping intentId and href assignments intact (refer to intentId, services, href and parsed.data.attributes.services in the surrounding method) so you only access the property once for clarity and slight perf improvement.
58-62: ⚡ Quick winConsider validating that access_token is non-empty.
While the code validates that
access_tokenis a String (line 59), it doesn't check whether it's empty. An empty access token would fail authorization downstream.🔍 Proposed validation
final accessToken = response.data['access_token']; -if (accessToken is! String) { +if (accessToken is! String || accessToken.isEmpty) { throw StateError('Invalid token response: access_token is ${accessToken.runtimeType}'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart` around lines 58 - 62, The current validation in workplace_datasource_impl.dart only checks that accessToken is a String but not that it's non-empty; update the validation in the method where accessToken is extracted (the block that assigns final accessToken = response.data['access_token']) to also verify accessToken.trim().isNotEmpty and throw a StateError with a clear message (e.g., "Invalid token response: access_token is empty") if the string is empty, while preserving the existing type check and error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart`:
- Around line 58-62: The code assumes response.data is a Map when reading
response.data['access_token']; add a defensive type check before indexing:
verify response.data is a Map<String, dynamic> (or Map) and throw a clear
StateError if it isn't, then proceed to extract accessToken and validate it is a
String as currently done (update the checks around response.data, accessToken
and the existing runtimeType error message in the same method in
workplace_datasource_impl.dart).
- Around line 18-33: The POST using _dio.post(...) to '$platformUrl/intents'
currently has no timeouts and can hang; add appropriate timeout settings
(connectTimeout/sendTimeout/receiveTimeout) to the request Options (or create a
RequestOptions/Options with .sendTimeout/.receiveTimeout/.connectTimeout) so the
call will fail fast and be handled by DioNoResponseErrorHandler; update the call
that sends WorkplaceIntentRequest.toJson() (the _dio.post invocation) to include
these timeout values in its Options headers/timeout configuration.
- Around line 34-38: The current parsing in WorkplaceDatasourceImpl uses
WorkplaceIntentResponse.fromJson with a fallback that calls jsonDecode on
response.data regardless of its type; change this to only call jsonDecode when
response.data is a String (e.g., response.data is Map<String, dynamic> ?
response.data : response.data is String ? jsonDecode(response.data) :
throw/handle unexpected type) so you don't attempt to jsonDecode non-String
already-parsed JSON objects; update the parsing logic around
WorkplaceIntentResponse.fromJson and response.data accordingly.
In `@workplace/lib/domain/message/workplace_intent_message.dart`:
- Around line 5-15: Wrap json parsing in parseDriveIntentMessage in a try-catch
that catches FormatException/TypeError and returns a
WorkplaceIntentUnknownMessage (include the raw or parsed type when possible)
instead of allowing a crash, and update WorkplaceIntentDoneMessage.fromJson to
perform safe validations and casts (check map entries with `is` checks or
null-aware conversions for fields like the type, id and file list, verify
elements' types before casting, and throw a controlled FormatException or return
a fallback) so malformed shapes are handled gracefully and propagate a
predictable unknown/failure message back to parseDriveIntentMessage.
In `@workplace/lib/domain/state/workplace_intent_state.dart`:
- Around line 15-20: The constructors of CreateWorkplaceIntentFailure and
ExchangeWorkplaceTokenFailure currently accept super.exception without an
explicit type, leaving it as dynamic; update both constructors to declare the
parameter type as Object? (e.g., CreateWorkplaceIntentFailure({Object?
exception}) and ExchangeWorkplaceTokenFailure({Object? exception})) so they pass
a statically typed exception to the FeatureFailure base class and keep the props
getters unchanged (returning [exception]).
---
Nitpick comments:
In `@pubspec.yaml`:
- Line 20: The workspace entry and dependency path currently reference the
folder name "workplace" but the package inside is named "drive_attachment", so
rename the folder from workplace/ to drive_attachment/ and update all references
in pubspec.yaml: replace the workspace entry "workplace" and any dependency path
"workplace" with "drive_attachment" (also update the dependency path under the
package that declared the path), ensuring the workspace list and path
dependencies match the package name "drive_attachment" used in the package's
pubspec.yaml.
In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart`:
- Around line 39-44: The code reads parsed.data.attributes.services twice;
extract parsed.data.attributes.services into a local variable (e.g., services)
and use that variable for the emptiness check and to get services.first.href,
keeping intentId and href assignments intact (refer to intentId, services, href
and parsed.data.attributes.services in the surrounding method) so you only
access the property once for clarity and slight perf improvement.
- Around line 58-62: The current validation in workplace_datasource_impl.dart
only checks that accessToken is a String but not that it's non-empty; update the
validation in the method where accessToken is extracted (the block that assigns
final accessToken = response.data['access_token']) to also verify
accessToken.trim().isNotEmpty and throw a StateError with a clear message (e.g.,
"Invalid token response: access_token is empty") if the string is empty, while
preserving the existing type check and error behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 249ade8b-98e4-43ba-94c2-ada585bf97ed
📒 Files selected for processing (19)
pubspec.yamlworkplace/lib/data/datasource/workplace_datasource.dartworkplace/lib/data/datasource_impl/workplace_datasource_impl.dartworkplace/lib/data/model/workplace_enums.dartworkplace/lib/data/model/workplace_exchange_token_request.dartworkplace/lib/data/model/workplace_intent_request.dartworkplace/lib/data/model/workplace_intent_response.dartworkplace/lib/data/repository_impl/workplace_repository_impl.dartworkplace/lib/domain/entity/drive_document.dartworkplace/lib/domain/entity/drive_document_extension.dartworkplace/lib/domain/entity/workplace_intent.dartworkplace/lib/domain/message/workplace_intent_message.dartworkplace/lib/domain/repository/workplace_repository.dartworkplace/lib/domain/state/workplace_intent_state.dartworkplace/lib/domain/usecase/create_drive_intent_interactor.dartworkplace/lib/domain/usecase/exchange_drive_token_interactor.dartworkplace/pubspec.yamlworkplace/test/domain/entity/drive_document_extension_test.dartworkplace/test/domain/message/workplace_intent_message_test.dart
✅ Files skipped from review due to trivial changes (2)
- workplace/lib/data/datasource/workplace_datasource.dart
- workplace/lib/data/repository_impl/workplace_repository_impl.dart
Comment on lines
+18
to
+33
| final response = await _dio.post( | ||
| '$platformUrl/intents', | ||
| options: Options( | ||
| headers: {'Authorization': 'Bearer $accessToken'}, | ||
| ), | ||
| data: const WorkplaceIntentRequest( | ||
| data: WorkplaceIntentDataRequest( | ||
| type: WorkplaceDataRequestType.intents, | ||
| attributes: WorkplaceIntentAttributesRequest( | ||
| action: WorkplaceAction.pick, | ||
| type: WorkplaceAttributesRequestType.files, | ||
| permissions: [WorkplacePermission.get], | ||
| ), | ||
| ), | ||
| ).toJson(), | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if Dio instance is configured with default timeouts elsewhere
# Search for Dio configuration in the workspace
rg -n "Dio\(" --type dart -A 5 -g 'workplace/**'
# Search for timeout configurations
rg -n "connectTimeout|receiveTimeout|sendTimeout" --type dart -g 'workplace/**'Repository: linagora/tmail-flutter
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where _dio is declared/initialized in the specific file
sed -n '1,120p' workplace/lib/data/datasource_impl/workplace_datasource_impl.dart 2>/dev/null || true
rg -n "final Dio _dio|Dio _dio|_dio" workplace/lib/data/datasource_impl/workplace_datasource_impl.dart -S || true
# 2) Search the whole repo for Dio instantiation/config (timeouts + BaseOptions)
rg -n "connectTimeout|receiveTimeout|sendTimeout|BaseOptions\\(|timeout:" --type dart . -S || true
# 3) Find Dio construction patterns (factories/providers/singletons)
rg -n "\\bDio\\b" --type dart . -S || true
# 4) If any Dio factory/provider is found, narrow search around its config
rg -n "Dio\\(" --type dart . -S -g'*.dart' || trueRepository: linagora/tmail-flutter
Length of output: 19201
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show how BaseOptions/Dio is created in production bindings
for f in lib/main/bindings/network/network_bindings.dart lib/main/bindings/network/network_isolate_binding.dart core/lib/data/network/dio_client.dart; do
echo "===== $f ====="
sed -n '1,220p' "$f" | cat -n
done
# Targeted search for timeout settings in lib/ only (exclude tests)
echo "===== timeout search in lib/ ====="
rg -n "connectTimeout|receiveTimeout|sendTimeout|\\btimeout:" --type dart lib -S || true
# Also check for BaseOptions timeout fields in lib/ only
echo "===== BaseOptions timeout search in lib/ ====="
rg -n "BaseOptions\\([^\\)]*(connectTimeout|receiveTimeout|sendTimeout|timeout|receiveTimeout|sendTimeout)" --type dart lib -S || trueRepository: linagora/tmail-flutter
Length of output: 19975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all BaseOptions constructions in lib/
rg -n "BaseOptions\\(" --type dart lib -S || true
# Look for connectTimeout/receiveTimeout/sendTimeout mutations anywhere in lib/
rg -n "connectTimeout|receiveTimeout|sendTimeout" --type dart lib -S || true
# Look for places that adjust Dio options (timeout-like) after creation
rg -n "_dio\\.options\\.|\\.options\\.(connectTimeout|receiveTimeout|sendTimeout|timeout)" --type dart lib -S || true
# Show where WorkplaceDataSourceImpl is created (to see what Dio it receives)
rg -n "WorkplaceDataSourceImpl\\(" --type dart . -S || trueRepository: linagora/tmail-flutter
Length of output: 487
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,200p' lib/main/exceptions/thrower/dio_no_response_error_handler.dart | cat -nRepository: linagora/tmail-flutter
Length of output: 3462
Add Dio timeouts to the Workplace POST request (avoid indefinite hangs).
In workplace/lib/data/datasource_impl/workplace_datasource_impl.dart (lines 18-33), the _dio.post(...) call has no timeout/connectTimeout/receiveTimeout/sendTimeout configured. The app-wide Dio is built from BaseOptions(headers: ...) only (no timeout settings in lib/), so a stalled server can hang the request thread indefinitely. The existing DioNoResponseErrorHandler already handles connectionTimeout/sendTimeout/receiveTimeout, so setting timeouts will route failures through the proper error handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart` around
lines 18 - 33, The POST using _dio.post(...) to '$platformUrl/intents' currently
has no timeouts and can hang; add appropriate timeout settings
(connectTimeout/sendTimeout/receiveTimeout) to the request Options (or create a
RequestOptions/Options with .sendTimeout/.receiveTimeout/.connectTimeout) so the
call will fail fast and be handled by DioNoResponseErrorHandler; update the call
that sends WorkplaceIntentRequest.toJson() (the _dio.post invocation) to include
these timeout values in its Options headers/timeout configuration.
Comment on lines
+34
to
+38
| final parsed = WorkplaceIntentResponse.fromJson( | ||
| response.data is Map<String, dynamic> | ||
| ? response.data | ||
| : jsonDecode(response.data), | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check Dio configuration for JSON parsing
# Search for Dio instantiation and configuration
rg -n "Dio|BaseOptions|ResponseType" --type dart -g 'workplace/**' -A 3Repository: linagora/tmail-flutter
Length of output: 400
🏁 Script executed:
#!/bin/bash
# Inspect Dio usage/config within workplace
set -euo pipefail
# 1) Show the datasource implementation around request/response handling
sed -n '1,120p' workplace/lib/data/datasource_impl/workplace_datasource_impl.dart
# 2) Find where Dio/BaseOptions/ResponseType are set anywhere under workplace/
rg -n "Dio\\(|BaseOptions|ResponseType" --type dart workplace -A 3
# 3) Find any jsonDecode usage related to Dio responses under workplace/
rg -n "jsonDecode\\(" --type dart workplace -A 3
# 4) Find the place where the Dio instance passed to WorkplaceDataSourceImpl is created
rg -n "WorkplaceDataSourceImpl\\(" --type dart workplace -A 5Repository: linagora/tmail-flutter
Length of output: 2299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where Dio is instantiated/configured anywhere in repo
rg -n "Dio\\(" --type dart -S . -g'*.dart' -A 2
rg -n "BaseOptions\\(" --type dart -S . -g'*.dart' -A 4
rg -n "ResponseType\\." --type dart -S . -g'*.dart' -A 3
# Trace how WorkplaceDataSourceImpl is wired (dependency injection / constructors)
rg -n "WorkplaceDataSourceImpl\\(" --type dart -S . -g'*.dart' -A 5Repository: linagora/tmail-flutter
Length of output: 29664
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show Dio/BaseOptions bindings in main
sed -n '1,140p' lib/main/bindings/network/network_bindings.dart
sed -n '1,120p' lib/main/bindings/network/network_isolate_binding.dart
# Locate any BaseOptions configuration (headers/mime/content-type/responseType)
rg -n "Get\.put\\(BaseOptions\\(" --type dart -S lib core workplace -g'*.dart'
rg -n "BaseOptions\\(" --type dart -S lib core workplace -g'*.dart'Repository: linagora/tmail-flutter
Length of output: 11884
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for explicit JSON responseType settings anywhere
rg -n "responseType\\s*:\\s*ResponseType\\.(json|plain|dynamic)|ResponseType\\.json" --type dart -S . -g'*.dart'
# Search for custom ResponseType/transformer setup in DioClient / network bindings
rg -n "Transformer|transformers|DefaultTransformer|jsonDecode\\(|ResponseType\\." --type dart -S lib core workplace -g'*.dart'
# Specifically look for BaseOptions responseType assignments
rg -n "BaseOptions\\([^\\)]*responseType" --type dart -S . -g'*.dart'Repository: linagora/tmail-flutter
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show DioClient implementation
sed -n '1,220p' core/lib/data/network/dio_client.dart
# Also inspect any HTTP client wrapper that might set options/transformers
rg -n "class DioClient|Transformer|transformers|defaultTransformer|responseType" --type dart core/lib lib -S -g'*.dart'
# Search for explicit Options(...) usage on the shared Dio for JSON
rg -n "_dio\\.post\\(|_dio\\.get\\(|Dio\\(Get\\.find<BaseOptions>\\)" --type dart -S workplace lib core -g'*.dart'
# Find Constant defaults to see what content-type header is set to
rg -n "class Constant|acceptHeaderDefault|contentTypeHeaderDefault" -S lib core -g'*.dart'Repository: linagora/tmail-flutter
Length of output: 30798
🌐 Web query:
Dio default ResponseType json response.data type Map string
💡 Result:
In the Dio library, when the response type is set to the default ResponseType.json, Dio automatically parses the response body into a Dart object [1][2]. The resulting type of response.data is typically a Map<String, dynamic> (for JSON objects) or a List (for JSON arrays), provided the server returns a valid "application/json" content type [3][4]. Key points regarding Dio's JSON handling: 1. Automatic Transformation: Because ResponseType.json is the default, you do not need to manually call json.decode on the response data [1][5]. Doing so will often cause an error because the data has already been transformed into a Dart object [5]. 2. Accessing the Data: Since the response data is already a collection (Map or List), you can access it directly [5]. For example, if the API returns a JSON object, you can access values using response.data['key'] [5]. 3. Type Safety: When making requests, you can use generics to inform Dio of the expected return type, such as await dio.get<Map<String, dynamic>>('/url') [6]. However, note that if the actual response structure does not match your generic type at runtime, you may encounter a type casting error [7]. If you explicitly need the raw string instead of the parsed Map or List, you should set the responseType to ResponseType.plain in your request options [3][1].
Citations:
- 1: https://pub.dev/documentation/dio/latest/index.html
- 2: https://github.com/cfug/dio/blob/main/dio/README.md
- 3: https://pub.dev/documentation/dio/latest/dio/ResponseType.html
- 4: https://pub.dev/documentation/dio_http/latest/dio_http/ResponseType.html
- 5: https://stackoverflow.com/questions/54366431/dio-response-decode-issue
- 6: https://pub.dev/documentation/dio/latest/dio/DioMixin/get.html
- 7: Type Casting Error: '_Map<String, dynamic>' is not a Subtype of 'T?' cfug/dio#2231
Dio already parses JSON by default; make the jsonDecode fallback safer
NetworkBindings creates the shared Dio as Dio(Get.find<BaseOptions>()) without overriding responseType, so Dio’s default ResponseType.json should already decode JSON responses into Dart objects (Map/List). The jsonDecode(response.data) fallback is likely unnecessary and can become incorrect if response.data is already a non-String JSON type.
final parsed = WorkplaceIntentResponse.fromJson(
response.data is Map<String, dynamic>
? response.data
: jsonDecode(response.data),
);If keeping the fallback, only decode when response.data is String (otherwise fail fast / handle other shapes).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart` around
lines 34 - 38, The current parsing in WorkplaceDatasourceImpl uses
WorkplaceIntentResponse.fromJson with a fallback that calls jsonDecode on
response.data regardless of its type; change this to only call jsonDecode when
response.data is a String (e.g., response.data is Map<String, dynamic> ?
response.data : response.data is String ? jsonDecode(response.data) :
throw/handle unexpected type) so you don't attempt to jsonDecode non-String
already-parsed JSON objects; update the parsing logic around
WorkplaceIntentResponse.fromJson and response.data accordingly.
Comment on lines
+58
to
+62
| final accessToken = response.data['access_token']; | ||
| if (accessToken is! String) { | ||
| throw StateError('Invalid token response: access_token is ${accessToken.runtimeType}'); | ||
| } | ||
| return accessToken; |
Contributor
There was a problem hiding this comment.
Add defensive check for response.data type.
Line 58 directly accesses response.data['access_token'] without verifying that response.data is a Map. If the server returns an unexpected response format, this will throw a runtime error.
🛡️ Proposed fix to add type check
-final accessToken = response.data['access_token'];
+final responseData = response.data;
+if (responseData is! Map<String, dynamic>) {
+ throw StateError('Invalid token response: expected Map but got ${responseData.runtimeType}');
+}
+final accessToken = responseData['access_token'];
if (accessToken is! String) {
throw StateError('Invalid token response: access_token is ${accessToken.runtimeType}');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final accessToken = response.data['access_token']; | |
| if (accessToken is! String) { | |
| throw StateError('Invalid token response: access_token is ${accessToken.runtimeType}'); | |
| } | |
| return accessToken; | |
| final responseData = response.data; | |
| if (responseData is! Map<String, dynamic>) { | |
| throw StateError('Invalid token response: expected Map but got ${responseData.runtimeType}'); | |
| } | |
| final accessToken = responseData['access_token']; | |
| if (accessToken is! String) { | |
| throw StateError('Invalid token response: access_token is ${accessToken.runtimeType}'); | |
| } | |
| return accessToken; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workplace/lib/data/datasource_impl/workplace_datasource_impl.dart` around
lines 58 - 62, The code assumes response.data is a Map when reading
response.data['access_token']; add a defensive type check before indexing:
verify response.data is a Map<String, dynamic> (or Map) and throw a clear
StateError if it isn't, then proceed to extract accessToken and validate it is a
String as currently done (update the checks around response.data, accessToken
and the existing runtimeType error message in the same method in
workplace_datasource_impl.dart).
Comment on lines
+5
to
+15
| WorkplaceIntentMessage parseDriveIntentMessage(String intentId, String raw) { | ||
| final map = jsonDecode(raw) as Map<String, dynamic>; | ||
| final type = map['type'] as String? ?? ''; | ||
| return switch (type) { | ||
| _ when type == 'intent-$intentId:ready' => const WorkplaceIntentReadyMessage(), | ||
| _ when type == 'intent-$intentId:done' => WorkplaceIntentDoneMessage.fromJson(map), | ||
| _ when type == 'intent-$intentId:error' => const WorkplaceIntentErrorMessage(), | ||
| _ when type == 'intent-$intentId:cancel' => const WorkplaceIntentCancelMessage(), | ||
| _ => WorkplaceIntentUnknownMessage(type), | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Add error handling for malformed JSON and type mismatches.
Both parseDriveIntentMessage (line 6) and WorkplaceIntentDoneMessage.fromJson (lines 47-49) perform unsafe operations that can crash at runtime:
jsonDecode(raw)throwsFormatExceptionifrawis invalid JSON.- Type casts (
as String?,as List?,as Map<String, dynamic>) throwTypeErrorif the JSON structure doesn't match expectations.
Even if the server is expected to send well-formed data, network corruption, server bugs, or future API changes could produce malformed input. Crashing the app degrades user experience.
🛡️ Proposed fix: wrap parsing in try-catch and return unknown message on error
For parseDriveIntentMessage:
WorkplaceIntentMessage parseDriveIntentMessage(String intentId, String raw) {
+ try {
final map = jsonDecode(raw) as Map<String, dynamic>;
final type = map['type'] as String? ?? '';
return switch (type) {
_ when type == 'intent-$intentId:ready' => const WorkplaceIntentReadyMessage(),
_ when type == 'intent-$intentId:done' => WorkplaceIntentDoneMessage.fromJson(map),
_ when type == 'intent-$intentId:error' => const WorkplaceIntentErrorMessage(),
_ when type == 'intent-$intentId:cancel' => const WorkplaceIntentCancelMessage(),
_ => WorkplaceIntentUnknownMessage(type),
};
+ } catch (e) {
+ return WorkplaceIntentUnknownMessage('parse-error');
+ }
}For WorkplaceIntentDoneMessage.fromJson, the try-catch above will already handle errors. Alternatively, add safe casting inside:
factory WorkplaceIntentDoneMessage.fromJson(Map<String, dynamic> json) =>
WorkplaceIntentDoneMessage(
- (json['document'] as List? ?? [])
+ ((json['document'] is List ? json['document'] as List : []) ?? [])
.map((e) => DriveDocument.fromJson(e as Map<String, dynamic>))
.toList(),
);Or wrap the entire map call in a try-catch to gracefully handle malformed documents.
Also applies to: 45-50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workplace/lib/domain/message/workplace_intent_message.dart` around lines 5 -
15, Wrap json parsing in parseDriveIntentMessage in a try-catch that catches
FormatException/TypeError and returns a WorkplaceIntentUnknownMessage (include
the raw or parsed type when possible) instead of allowing a crash, and update
WorkplaceIntentDoneMessage.fromJson to perform safe validations and casts (check
map entries with `is` checks or null-aware conversions for fields like the type,
id and file list, verify elements' types before casting, and throw a controlled
FormatException or return a fallback) so malformed shapes are handled gracefully
and propagate a predictable unknown/failure message back to
parseDriveIntentMessage.
Comment on lines
+15
to
+20
| class CreateWorkplaceIntentFailure extends FeatureFailure { | ||
| CreateWorkplaceIntentFailure({super.exception}); | ||
|
|
||
| @override | ||
| List<Object?> get props => [exception]; | ||
| } |
Contributor
There was a problem hiding this comment.
Type exception parameter as Object? instead of inheriting dynamic.
The failure classes use super.exception without an explicit type annotation, inheriting dynamic from the base FeatureFailure class. Based on learnings, FeatureFailure subclasses should explicitly type the exception parameter as Object? to follow Effective Dart guidance and avoid disabling static type checking.
🔧 Proposed fix
class CreateWorkplaceIntentFailure extends FeatureFailure {
- CreateWorkplaceIntentFailure({super.exception});
+ CreateWorkplaceIntentFailure({Object? super.exception});
`@override`
List<Object?> get props => [exception];
}Apply the same change to ExchangeWorkplaceTokenFailure:
class ExchangeWorkplaceTokenFailure extends FeatureFailure {
- ExchangeWorkplaceTokenFailure({super.exception});
+ ExchangeWorkplaceTokenFailure({Object? super.exception});
`@override`
List<Object?> get props => [exception];
}Also applies to: 32-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workplace/lib/domain/state/workplace_intent_state.dart` around lines 15 - 20,
The constructors of CreateWorkplaceIntentFailure and
ExchangeWorkplaceTokenFailure currently accept super.exception without an
explicit type, leaving it as dynamic; update both constructors to declare the
parameter type as Object? (e.g., CreateWorkplaceIntentFailure({Object?
exception}) and ExchangeWorkplaceTokenFailure({Object? exception})) so they pass
a statically typed exception to the FeatureFailure base class and keep the props
getters unchanged (returning [exception]).
Source: Learnings
tddang-linagora
force-pushed
the
feat/TF-4449-drive-attachment-data
branch
from
21253a7 to
50a7cb1
Compare
tddang-linagora
had a problem deploying
to
PR-4587
GitHub Actions
Failure
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
tddang-linagora
changed the base branch from
master
to
feature/TF-4449-attach-from-drive
tddang-linagora
had a problem deploying
to
PR-4587
GitHub Actions
Failure
tddang-linagora
force-pushed
the
feat/TF-4449-drive-attachment-data
branch
from
50a7cb1 to
1dd79d0
Compare
tddang-linagora
changed the base branch from
feature/TF-4449-attach-from-drive
to
feat/TF-4583-drive-attachment-button-visibility
tddang-linagora
force-pushed
the
feat/TF-4583-drive-attachment-button-visibility
branch
from
d7cb790 to
2b6c792
Compare
tddang-linagora
force-pushed
the
feat/TF-4449-drive-attachment-data
branch
from
1dd79d0 to
adf8cd3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
tddang-linagora
had a problem deploying
to
PR-4587
GitHub Actions
Failure
tddang-linagora
force-pushed
the
feat/TF-4583-drive-attachment-button-visibility
branch
from
2b6c792 to
2bfb103
Compare
Add new drive_attachment Flutter package with data and domain layers: - Data: datasource interface/impl, models (DriveIntentRequest/Response/Enums) - Domain: entities (DriveAttachment, DriveDocument, DriveIntent), repository interface, states, use cases (CreateDriveIntent, ExchangeDriveToken), DriveIntentMessage for JS interop - Register drive_attachment in workspace and as app dependency - Tests: DriveDocumentExtension, DriveIntentMessage
tddang-linagora
force-pushed
the
feat/TF-4449-drive-attachment-data
branch
from
adf8cd3 to
15b6ddc
Compare
tddang-linagora
had a problem deploying
to
PR-4587
GitHub Actions
Failure
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
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
4 participants
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.
Issue
Description
Add new drive_attachment Flutter package with data and domain layers:
Summary by CodeRabbit
New Features
Tests
Chores