MessagesAction Update to also handle non-actionable messages without failing

[pauljohanneskraft]

MessagesAction Update to also handle non-actionable messages without failing

♻️ Current situation & Problem

Previously, the app would handle a non-existing action as a failure, whereas it can simply be ignored and a message should then either be dismissible or will be dismissed automatically based on other user input.

⚙️ Release Notes

Add a bullet point list summary of the feature and possible migration guides if this is a breaking change so this section can be added to the release notes.
Include code snippets that provide examples of the feature implemented or links to the documentation if it appends or changes the public interface.

📚 Documentation

Please ensure that you properly document any additions in conformance to Spezi Documentation Guide.
You can use this section to describe your solution, but we encourage contributors to document your reasoning and changes using in-line documentation.

✅ Testing

Please ensure that the PR meets the testing requirements set by CodeCov and that new functionality is appropriately tested.
This section describes important information about the tests and why some elements might not be testable.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

• I agree to follow the Code of Conduct and Contributing Guidelines.

[pauljohanneskraft]

I would like to discuss this further before merging, since there seem to be quite a few UI differences between messages on iOS and Android and I would rather discuss them before throwing out existing functionality here.

Examples:

• iOS has multi-line collapsing only after 3+ lines (by using a text button "Read more") and I'm not sure whether it is really intuitive there considering that if you tap on the collapsible message, the action is performed, so it is actually really hard to collapse and not perform the action instead. Android has that right chevron icon instead - We may want to think to just never collapse, right?

• iOS differentiates between dismissing and performing an action. Where you may dismiss a message on iOS without performing its action, you can only interact with a message using the "Finish" button on Android. Further, the "Finish" text is bound to the action type, which is not the case for Android.

• Due date is not displayed on iOS and may be removed on Android, since we do not set it on the server anymore (and if we do in some rare occasions, we do not need to visualize it).

• The isLoading property on Message shouldn't be in the model type but is rather UI-exclusive. Therefore, we could, for example, write a small wrapper type or something to add those UI-exclusive values.

[PSchmiedmayer]

@pauljohanneskraft Thank you for looking into this and thanks for the first feedback @eldcn!

Would be great to reduce the disparities between the different platforms.

iOS has multi-line collapsing only after 3+ lines (by using a text button "Read more") and I'm not sure whether it is really intuitive there considering that if you tap on the collapsible message, the action is performed, so it is actually really hard to collapse and not perform the action instead. Android has that right chevron icon instead - We may want to think to just never collapse, right?

I think it could be fine to never collapse a message. We might still want to add a truncation mark after e.g. 10 lines to ensure that a long text or a bug doesn't break the UI?

iOS differentiates between dismissing and performing an action. Where you may dismiss a message on iOS without performing its action, you can only interact with a message using the "Finish" button on Android. Further, the "Finish" text is bound to the action type, which is not the case for Android.

I like the way iOS is currently handling this. An X for any dismissible actions and a action button that is bound to the message type & let's you perform a message.

Due date is not displayed on iOS and may be removed on Android, since we do not set it on the server anymore (and if we do in some rare occasions, we do not need to visualize it).

Agree, we can remove that.

The isLoading property on Message shouldn't be in the model type but is rather UI-exclusive. Therefore, we could, for example, write a small wrapper type or something to add those UI-exclusive values.

Good point.

@eldcn & @pauljohanneskraft let me know if you need any additional context; happy to also sync on this on Tuesday if there are more open questions.

[codecov]

Codecov Report

Attention: Patch coverage is 49.50495% with 102 lines in your changes missing coverage. Please review.

Project coverage is 40.76%. Comparing base (e32d00a) to head (db62fcc).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../edu/stanford/bdh/engagehf/messages/MessageItem.kt	0.00%	56 Missing ⚠️
...u/stanford/bdh/engagehf/bluetooth/HomeViewModel.kt	68.75%	8 Missing and 7 partials ⚠️
...ford/bdh/engagehf/bluetooth/data/models/UiState.kt	55.56%	8 Missing ⚠️
.../stanford/bdh/engagehf/messages/MessagesHandler.kt	69.24%	4 Missing and 4 partials ⚠️
...du/stanford/bdh/engagehf/messages/MessageAction.kt	22.23%	7 Missing ⚠️
...anford/bdh/engagehf/bluetooth/screen/HomeScreen.kt	0.00%	6 Missing ⚠️
...edu/stanford/bdh/engagehf/MainActivityViewModel.kt	75.00%	0 Missing and 1 partial ⚠️
...agehf/bluetooth/data/mapper/MessageActionMapper.kt	90.91%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #168      +/-   ##
============================================
- Coverage     40.80%   40.76%   -0.03%     
- Complexity      866      867       +1     
============================================
  Files           299      299              
  Lines         11514    11553      +39     
  Branches       1736     1752      +16     
============================================
+ Hits           4697     4709      +12     
- Misses         6352     6373      +21     
- Partials        465      471       +6     

Flag	Coverage Δ
uitests	35.99% <ø> (ø)
unittests	32.25% <49.51%> (-<0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...nford/bdh/engagehf/bluetooth/data/models/Action.kt	100.00% <100.00%> (ø)
.../bottomsheet/AddBloodPressureBottomSheetUiState.kt	93.55% <ø> (ø)
.../bdh/engagehf/medication/ui/MedicationViewModel.kt	77.15% <100.00%> (ø)
...rd/bdh/engagehf/messages/FirestoreMessageMapper.kt	57.90% <100.00%> (+0.76%)	⬆️
...tlin/edu/stanford/bdh/engagehf/messages/Message.kt	100.00% <100.00%> (+43.91%)	⬆️
...tanford/bdh/engagehf/messages/MessageRepository.kt	79.25% <100.00%> (ø)
...edu/stanford/bdh/engagehf/MainActivityViewModel.kt	96.62% <75.00%> (-1.75%)	⬇️
...agehf/bluetooth/data/mapper/MessageActionMapper.kt	96.00% <90.91%> (ø)
...anford/bdh/engagehf/bluetooth/screen/HomeScreen.kt	0.00% <0.00%> (ø)
...du/stanford/bdh/engagehf/messages/MessageAction.kt	22.23% <22.23%> (ø)
... and 4 more

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e32d00a...db62fcc. Read the comment docs.

[PSchmiedmayer]

@pauljohanneskraft Double-check that this resolves #157 & #159