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(frontend): implements reward canister api #3996

Merged

Conversation

BonomoAlessandro
Copy link
Collaborator

@BonomoAlessandro BonomoAlessandro commented Dec 17, 2024

Motivation

We want to use the reward canister api and the reward service so that we can send some rewards to new or existing users.

Changes

  • implements reward canister api
  • implements reward service

@peterpeterparker
Copy link
Member

I randomly noticed this PR. Not sure if it's a general work in progress or effective implementation of a feature so just a heads-up: canister, API, and services should be provided in separate PRs, each covered by tests.

@BonomoAlessandro
Copy link
Collaborator Author

@peterpeterparker atm it's just work in progress. But good to know, if it is finished, i will split them into different PRs

@BonomoAlessandro BonomoAlessandro marked this pull request as ready for review December 18, 2024 16:10
@BonomoAlessandro BonomoAlessandro requested a review from a team as a code owner December 18, 2024 16:10
@BonomoAlessandro
Copy link
Collaborator Author

BonomoAlessandro commented Dec 18, 2024

@peterpeterparker now this PR is open and ready to be reviewed

Copy link
Member

@peterpeterparker peterpeterparker left a comment

Choose a reason for hiding this comment

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

LGTM, thx

Reviewed offline together with @BonomoAlessandro. Few minor improvements will be provided within this PR after this approval as discussed to move forward.

BonomoAlessandro and others added 3 commits January 6, 2025 16:21
adds a small doc to the update functions
…lements-reward-canister-api

# Conflicts:
#	src/frontend/src/lib/i18n/en.json
@BonomoAlessandro BonomoAlessandro merged commit a7244d6 into main Jan 6, 2025
22 checks passed
@BonomoAlessandro BonomoAlessandro deleted the feature(frontend)/implements-reward-canister-api branch January 6, 2025 15:26
peterpeterparker added a commit that referenced this pull request Jan 7, 2025
# Motivation

I reviewed the `reward-code.services` after merging #3996 and noticed a
few areas that could be better aligned with the codebase's style and
patterns for consistency.

# Changes for Consistency

- Add JSDoc comments for exported functions.  
- Use specific error classes instead of the generic `Error` when the
error type is known.
- Bubble the `err` in the `ResultSuccess` return to potentially inform
the consumer about which error occurred.
- Align the pattern of `updateVipReward` with the pattern of other
internal functions in the service, `updateReward`, which throws errors
instead of returning values.
- Define `err` as `err: unknown`.

# Opinionated Changes

- Destructure parameters.  

# TODO

- Add a TODO for `getNewReward` to use a `ResultSuccess` return type as
well.

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.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.

2 participants