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(Bonus Pagamenti Digitali): [#176318970] Avoid call bpd API if the user is not enrolled #2663

Merged

Conversation

fabriziofff
Copy link
Contributor

@fabriziofff fabriziofff commented Dec 30, 2020

Short description

This pr removes the call to the BPD API if the user is not enrolled to bpd ( atm the user should never see a closed period if is not enrolled to bpd anymore)

List of changes proposed in this pull request

  • Changed loadBpdData in order to avoid to call others bpd API if the user is not enrolled
  • Changed bpdLoadRaking in order to handle the return code 404 as an empty array of periods.
  • Changed enrollToBpdWorker in order to request the updated bpd data after the enroll of the user
  • Changed bpdPeriodsReducer in order to clean the periods state after the user unsubscribe from bpd.

Expected behaviours:

  • The user is not enrolled to bpd -> when load the bonus in the wallet -> only the api bpd/io/citizen is called
  • The user is enrolled to bpd -> when load the bonus in the wallet -> all the bpd api are called
  • The api bpd/io/citizen/rankingreturn 404 -> the user will see a "ranking not ready"
  • Run ts/features/bonus/bpd/saga/networking/__test__/loadBpdData.test.ts

@pagopa-github-bot pagopa-github-bot changed the title [#176318970] Avoid call bpd API if the user is not enrolled feat(Bonus Pagamenti Digitali): [#176318970] Avoid call bpd API if the user is not enrolled Dec 30, 2020
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Dec 30, 2020

Affected stories

  • 🌟 #176318970: Come DEV non voglio che siano invocate le api di ranking se il CIT non è iscritto al cashback

Generated by 🚫 dangerJS against d4ede1e

@codecov
Copy link

codecov bot commented Dec 30, 2020

Codecov Report

Merging #2663 (d4ede1e) into master (5d06537) will increase coverage by 0.05%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2663      +/-   ##
==========================================
+ Coverage   51.49%   51.54%   +0.05%     
==========================================
  Files         730      730              
  Lines       20724    20731       +7     
  Branches     3967     3967              
==========================================
+ Hits        10671    10686      +15     
+ Misses      10009    10001       -8     
  Partials       44       44              
Impacted Files Coverage Δ
ts/features/bonus/bpd/saga/networking/ranking.ts 58.82% <0.00%> (-3.68%) ⬇️
...res/bonus/bpd/store/reducers/onboarding/ongoing.ts 62.50% <ø> (+4.60%) ⬆️
...atures/bonus/bpd/store/reducers/details/periods.ts 60.52% <33.33%> (-2.34%) ⬇️
...s/bpd/saga/orchestration/onboarding/enrollToBpd.ts 47.82% <50.00%> (+0.20%) ⬆️
.../features/bonus/bpd/saga/networking/loadBpdData.ts 71.42% <100.00%> (+35.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d06537...d4ede1e. Read the comment docs.

@@ -34,6 +35,7 @@ function* enrollToBpdWorker() {
);

if (enrollResult.payload.enabled) {
yield put(bpdAllData.request());
Copy link
Contributor

@Undermaken Undermaken Dec 31, 2020

Choose a reason for hiding this comment

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

I guess this request creates a side effect:
bpdAllData makes few requests and it also asks for the citizen activation status. The response (since the citizen is enrolled) invalidates the onboarding state (it becomes false due to this condition )

I mainly noted this side effect on the 2nd CTA button of this screen (UI thinks the user is already enrolled)

before now
before now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tnx for the catch, solved in d4ede1e, removing the condition (legacy, not needed anymore)

…970-avoid-call-bpd-api-if-the-user-is-not-enrolled
…l-bpd-api-if-the-user-is-not-enrolled' into 176318970-avoid-call-bpd-api-if-the-user-is-not-enrolled
@fabriziofff fabriziofff requested a review from Undermaken January 4, 2021 10:46
@Undermaken Undermaken merged commit d0582ef into master Jan 4, 2021
@Undermaken Undermaken deleted the 176318970-avoid-call-bpd-api-if-the-user-is-not-enrolled branch January 4, 2021 12:08
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