Skip to content

Debug view: Add offerings section and purchasing capabilities#1186

Merged
tonidero merged 5 commits into
mainfrom
debug-view-2
Aug 3, 2023
Merged

Debug view: Add offerings section and purchasing capabilities#1186
tonidero merged 5 commits into
mainfrom
debug-view-2

Conversation

@tonidero

@tonidero tonidero commented Aug 1, 2023

Copy link
Copy Markdown
Contributor

Description

This PR adds the offerings section to the debug menu and allows to purchase packages, products and subscription options for those offerings.

Screen_recording_20230801_110036.mp4

@tonidero tonidero added the test label Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could listen to purchase completion/errors in the composable... But felt that it would be better to move that logic to the viewModel. In order to pass the callbacks as parameters to the view model, we need to create a factory like this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the preview, I'm just creating a dummy activity. Shouldn't matter as long as the purchase buttons are not pressed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here I'm assuming there will always be an activity... I guess we could handle this and fail gracefully, but the debug view should always be used within a compose project that has an activity, and this is only for debug, so I thought it was ok. Lmk if you think otherwise.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine. Maybe a more descriptive message could help

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm showing the toast as part of the debug view. I think it's ok, but I was wondering whether it would be better to just leave to the developer to handle it as they see fit, since there is a callback. Thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I'm not sure if this is too many options for a public debug view... But I thought it could be useful to visualize the different subscription options.

@tonidero tonidero marked this pull request as ready for review August 1, 2023 09:50
@tonidero tonidero requested a review from a team August 1, 2023 09:51
@codecov

codecov Bot commented Aug 1, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1186 (5ec4c4f) into main (b428869) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head 5ec4c4f differs from pull request most recent head 9775e9e. Consider uploading reports for the commit 9775e9e to get more accurate results

@@           Coverage Diff           @@
##             main    #1186   +/-   ##
=======================================
  Coverage   85.92%   85.92%           
=======================================
  Files         181      181           
  Lines        6216     6216           
  Branches      899      899           
=======================================
  Hits         5341     5341           
  Misses        533      533           
  Partials      342      342           

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine. Maybe a more descriptive message could help

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we concerned these logs won't go through the logHandler if there's one configured? I don't think there's anything we can do anyways

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's not ideal... But not sure of a solution... We would need to expose something in the SDK to allow us to hook these logs to the same handler, and not sure it's worth it...

Personally, I think it's fine for now, and we can revisit later if needed, but lmk if you want to chat about it more.

Base automatically changed from debug-view to main August 3, 2023 09:58
@tonidero tonidero enabled auto-merge (squash) August 3, 2023 10:00
@tonidero tonidero merged commit e9664f0 into main Aug 3, 2023
@tonidero tonidero deleted the debug-view-2 branch August 3, 2023 10:15
@NachoSoto

Copy link
Copy Markdown
Contributor

This is great!!

tonidero pushed a commit that referenced this pull request Aug 10, 2023
**This is an automatic release.**

### Bugfixes
* Fix NoSuchElementException by using poll when accessing
serviceRequests (#1190) via Cesar de la Vega (@vegaro)
### Other Changes
* Debug view: rename package (#1191) via Toni Rico (@tonidero)
* Debugview: Add snapshot tests for debug view using Paparazzi (#1187)
via Toni Rico (@tonidero)
* Debug view: Add offerings section and purchasing capabilities (#1186)
via Toni Rico (@tonidero)
* Debug view: Initial UI + Usage in MagicWeatherCompose (#1075) via Toni
Rico (@tonidero)
* Remove customEntitlementComputation flavor for non-purchases modules
(#1180) via Toni Rico (@tonidero)

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
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