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

chore(points): Use activityId and pointsAmount throughout #5255

Merged
merged 30 commits into from
Apr 24, 2024

Conversation

jophish
Copy link
Contributor

@jophish jophish commented Apr 15, 2024

Description

Updates the points implementation + redux to use activityId and pointsAmount everywhere, as discussed in Slack.

Test plan

Manual/unit tested.

Related issues

Part of ACT-1032.

Backwards compatibility

Yes.

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

  • Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

Copy link

codecov bot commented Apr 15, 2024

Codecov Report

Attention: Patch coverage is 93.54839% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 86.08%. Comparing base (d99aa51) to head (00a571d).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5255      +/-   ##
==========================================
- Coverage   86.08%   86.08%   -0.01%     
==========================================
  Files         747      747              
  Lines       30671    30675       +4     
  Branches     5313     5314       +1     
==========================================
+ Hits        26403    26406       +3     
- Misses       4036     4037       +1     
  Partials      232      232              
Files Coverage Δ
src/analytics/Properties.tsx 100.00% <ø> (ø)
src/points/ActivityCard.tsx 100.00% <100.00%> (ø)
src/points/PointsHistoryBottomSheet.tsx 100.00% <100.00%> (ø)
src/points/PointsHome.tsx 100.00% <100.00%> (ø)
src/points/cardDefinitions.tsx 100.00% <ø> (ø)
src/points/saga.ts 88.57% <100.00%> (ø)
src/points/slice.ts 68.18% <ø> (ø)
src/points/types.ts 100.00% <100.00%> (ø)
src/redux/migrations.ts 97.03% <100.00%> (+<0.01%) ⬆️
src/redux/store.ts 78.68% <ø> (ø)
... and 3 more

... and 1 file with indirect coverage changes


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 d99aa51...00a571d. Read the comment docs.

Copy link
Collaborator

@kathaypacific kathaypacific left a comment

Choose a reason for hiding this comment

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

💅🏼🚀

@@ -11,26 +11,26 @@ import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import { PointsEvents } from 'src/analytics/Events'

interface Props {
activity: PointsActivity
activityId: PointsActivity
points: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this also be pointsAmount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, good call!

Base automatically changed from jophish/ret-1032-2 to main April 23, 2024 19:12
Copy link

emerge-tools bot commented Apr 23, 2024

📸 Snapshot Test

No snapshots generated

Name Version Added Removed Modified Unchanged Errored Approval
Celo (test)
org.celo.mobile.test
1.83.0 (148) 0 0 0 0 0 N/A

🛸 Powered by Emerge Tools

@jophish jophish added this pull request to the merge queue Apr 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 24, 2024
@jophish jophish added this pull request to the merge queue Apr 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 24, 2024
@jophish jophish added this pull request to the merge queue Apr 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 24, 2024
@jophish jophish added this pull request to the merge queue Apr 24, 2024
Merged via the queue into main with commit ac6b717 Apr 24, 2024
16 checks passed
@jophish jophish deleted the jophish/ret-1032-3 branch April 24, 2024 16:57
shottah pushed a commit to zed-io/kolektivo that referenced this pull request May 15, 2024
…#5255)

### Description

Updates the points implementation + redux to use `activityId` and
`pointsAmount` everywhere, as discussed in Slack.

### Test plan

Manual/unit tested.

### Related issues

Part of
[ACT-1032](https://linear.app/valora/issue/RET-1032/[wallet-fake-backend]-points-activity-history).

### Backwards compatibility

Yes.

### Network scalability

If a new NetworkId and/or Network are added in the future, the changes
in this PR will:

- [x] Continue to work without code changes, OR trigger a compilation
error (guaranteeing we find it when a new network is added)
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.

None yet

3 participants