Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GUI] Notify purchase #260

Merged
merged 17 commits into from
Sep 12, 2023
Merged

[GUI] Notify purchase #260

merged 17 commits into from
Sep 12, 2023

Conversation

CarlosNihelton
Copy link
Contributor

@CarlosNihelton CarlosNihelton commented Sep 5, 2023

This follows on the PR #246 adding the gRPC call in the client side to notify the agent of a successful purchase.

The SubscribeNowPage is now capable of displaying feedback according to the outcome of that transaction. A callback argument will be called if the transaction completes successfully. That allows the upper level to react, by updating the app's ValueNotifier<SusbcriptionInfo> object, effectively causing this page to leave the stage for a page matching the new SubscriptinInfo, which is production must be the MS Store managed subscription status page.

The UI element chosen for feedback on error is the SnackBar. Yaru changes it's style a little bit, so don't feel surprised by the rounded widget.

One integration test case was added to exercise that feedback. The "successful" test case cannot yet complete, because the background agent crashes when attempting to talk to the MS API. That will only be definitively addressed when the MS Store mock becomes available. On the GUI side we mock the binary messenger, so all Dart code is exercised when attempting to talk to the MS Store plugin, but the native code is skipped. Same as before.

Some screenshots of how it looks:

Captura de tela 2023-09-11 152948

Captura de tela 2023-09-11 152931

Captura de tela 2023-09-11 152912

Captura de tela 2023-09-11 152809

This one omits the native purchase dialog, because it's just a simulation, not a real purchase.
ubuntupro_zJSNQyhyPo

Fixes UDENG-1254

Otherwise clients would need two imports to make use of this plugin:

```dart
import 'package:p4w_ms_store/p4w_ms_store.dart';
import 'package:p4w_ms_store/p4w_ms_store_platform_interface.dart';
```
Silly to call AppLocalizations.of(context) if we already did that before
and even saved it in a variable.
Now it:
- notifies the agent on success;
- returns the purchase status on error;
- catches generic Exception's;
Snackbars are shown when the purchase completes
A callback is offered for when the purchase is sucessful.
Tests exercise both ways
The callback will notify the top level app of the new SubscriptionInfo.
That causes a complete page rebuild.
Going to the page matching the MS Store managed subscription status.

On error nothing will change.
The service locator state is kept between test cases.
The change is too abrupt otherwise.
It would appear together with the new screen.
Thus useless.
Since it's a callback after the purchase completes.
The user will be already subscribed.
@CarlosNihelton CarlosNihelton force-pushed the notify-purchase-udeng-1254 branch 4 times, most recently from 5823e56 to da5b965 Compare September 6, 2023 15:22
It appears that the agent is crashing.
GrpcError might be thrown before post-morten tests complete.
Integration tests report error when such happens.
@CarlosNihelton CarlosNihelton force-pushed the notify-purchase-udeng-1254 branch from da5b965 to a269734 Compare September 6, 2023 15:25
@CarlosNihelton
Copy link
Contributor Author

For now I'm amending the fact that the agent may crash after starting with just a try-catch, so we can succeed with the integration tests. I captured another task to properly handle the possibility of a generic agent crash.

@@ -25,7 +28,24 @@ class SubscribeNowPage extends StatelessWidget {
mainAxisAlignment: MainAxisAlignment.start,
children: [
ElevatedButton(
onPressed: model.purchaseSubscription,
onPressed: () async {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here...

// - Display errors
Future<void> purchaseSubscription() async {
await P4wMsStore().purchaseSubscription('9P25B50XMKXT');
/// Triggers a purchase transaction via MS Store.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and here are the motto for this PR.

All the rest is pretty much dealing with the consequences of those additions: adding more test cases, updating the existing ones, regenerating mocks, adding more localizable strings etc.

@CarlosNihelton CarlosNihelton marked this pull request as ready for review September 6, 2023 16:47
Copy link
Member

@d-loose d-loose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI implementation LGTM 👍
My only complaint is that the snack bar looks a bit ugly and could use some horizontal padding, but I'm no designer so feel free to ignore my opinion :)

Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments

@CarlosNihelton
Copy link
Contributor Author

CarlosNihelton commented Sep 11, 2023

UI implementation LGTM 👍 My only complaint is that the snack bar looks a bit ugly and could use some horizontal padding, but I'm no designer so feel free to ignore my opinion :)

It looks nicer indeed (I even updated the screenshots). I cannot go too far with padding, though, otherwise the SnackBar wraps the text around and that doesn't look pretty.

Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@d-loose
Copy link
Member

d-loose commented Sep 12, 2023

It looks nicer indeed (I even updated the screenshots). I cannot go too far with padding, though, otherwise the SnackBar wraps the text around and that doesn't look pretty.

Looks great!

@CarlosNihelton CarlosNihelton merged commit 287d7bb into main Sep 12, 2023
30 checks passed
@CarlosNihelton CarlosNihelton deleted the notify-purchase-udeng-1254 branch September 12, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants