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

feat: adds Product selection when using ticket #567

Merged
merged 5 commits into from
Jan 28, 2024
Merged

Conversation

marfavi
Copy link
Member

@marfavi marfavi commented Jan 20, 2024

Add support for using concrete products when claiming a ticket:

  • Upgraded ticket use flow to accommodate the selection of menu items explicitly.
  • The last selected menu item is cached locally for each ticket (cache cleared on logout).

other:

  • Simplified the Product feature file structure.
  • Add improvements to network request handling by adopting a TaskEither type for better composability.
  • Added package Hive for local storage
  • Rename some files and refactor several imports across the codebase
  • Fixed various minor bugs, tightened type constraints, and made use of pattern matching & fpdart types.

@ghost
Copy link

ghost commented Jan 20, 2024

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

main:
Add support for using concrete products when claiming a ticket
-> Upgraded ticket use flow to accommodate the selection of menu items explicitly.
-> The last selected menu item is cached locally for each ticket (cache cleared on logout).

other:
Simplified the Product feature file structure.
Add improvements to network request handling by adopting a TaskEither type for better composability.
Added package Hive for local storage
Rename some files and refactor several imports across the codebase
Fixed various minor bugs, tightened type constraints, and made use of pattern matching & fpdart types.
Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@242f176). Click here to learn what that means.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #567   +/-   ##
=======================================
  Coverage        ?   68.76%           
=======================================
  Files           ?      133           
  Lines           ?     1697           
  Branches        ?        0           
=======================================
  Hits            ?     1167           
  Misses          ?      530           
  Partials        ?        0           
Flag Coverage Δ
unittests 68.76% <0.00%> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marfavi marfavi changed the title Adds Product selection when using ticket feat: adds Product selection when using ticket Jan 25, 2024
@marfavi marfavi marked this pull request as ready for review January 25, 2024 11:44
Copy link
Member

@TTA777 TTA777 left a comment

Choose a reason for hiding this comment

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

There are frankly so many changes, I don't follow all of it.

With that said, only some smaller comments, where I would like some thoughts from you.

Also, are you sure that caching the last used menu item on swiping will have the intended outcome? I'm thinking that people will tend to be lazy, and just use whatever it was already on when swiping. This would be a shame, since it would basically make the data we get from it worthless. Is this something you have considered?

@marfavi
Copy link
Member Author

marfavi commented Jan 26, 2024

There are frankly so many changes, I don't follow all of it.

With that said, only some smaller comments, where I would like some thoughts from you.

Also, are you sure that caching the last used menu item on swiping will have the intended outcome? I'm thinking that people will tend to be lazy, and just use whatever it was already on when swiping. This would be a shame, since it would basically make the data we get from it worthless. Is this something you have considered?

It was implemented to help people feel like not much extra work is being done. We can ask baristas about how precise customer swipes 1-2 weeks in with this feature enabled.

@marfavi marfavi enabled auto-merge (squash) January 27, 2024 16:03
@marfavi marfavi merged commit 65c1fb7 into main Jan 28, 2024
9 of 10 checks passed
@marfavi marfavi deleted the ticket-menu-item branch January 28, 2024 18:20
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.

2 participants