feat: [#176825286,#176323469] Features Carousel: Items are now shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

CrisTofani · 2021-02-08T11:23:36Z

Short description

This PR converts the usage of the availableBonusTypesSelector with the new visibleAvailableBonusSelector which handles the visibility of bonus items

pagopa-github-bot · 2021-02-08T11:24:17Z

	Warnings
⚠️	Multiple stories with different types are associated with this Pull request. Only one tag will be added, following the order: `feature > bug > chore`
⚠️	Different scopes were found on the stories related to the pull request: [Carta Giovani Nazionale,Bonus Pagamenti Digitali]. `It is not possible to assign a single scope to this pull request!`

Affected stories

🌟 #176825286: L'item nel carosello deve basarsi sul FF remoto della lista dei bonus disponibili
🐞 #176323469: [trivial] il carosello nel portafoglio si mostra prima che l'informazione sia arrivata

Generated by 🚫 dangerJS against 7a8beb0

codecov · 2021-02-08T11:27:51Z

Codecov Report

Merging #2793 (7a8beb0) into master (a286f21) will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #2793      +/-   ##
==========================================
+ Coverage   54.83%   54.85%   +0.01%     
==========================================
  Files         813      813              
  Lines       22547    22547              
  Branches     4233     4233              
==========================================
+ Hits        12364    12368       +4     
+ Misses      10122    10118       -4     
  Partials       61       61

Impacted Files	Coverage Δ
...features/wallet/component/FeaturedCardCarousel.tsx	`40.98% <25.00%> (-0.69%)`	⬇️
...screens/onboarding/BpdCTAStartOnboardingScreen.tsx	`63.63% <50.00%> (-0.37%)`	⬇️
...onus/bonusVacanze/screens/AvailableBonusScreen.tsx	`47.05% <100.00%> (+1.22%)`	⬆️
...nusVacanze/store/reducers/availableBonusesTypes.ts	`76.59% <100.00%> (+4.09%)`	⬆️
ts/screens/wallet/WalletHomeScreen.tsx	`25.11% <100.00%> (+0.11%)`	⬆️
ts/utils/pot.ts	`100.00% <100.00%> (ø)`

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 a286f21...7a8beb0. Read the comment docs.

…sage

Undermaken · 2021-02-11T09:14:07Z

ts/features/bonus/bonusVacanze/store/reducers/availableBonusesTypes.ts

+  (abs: AvailableBonusTypesState) => pot.isLoading(abs)
+);
+
+export const isAvailableBonusNoneErrorSelector = createSelector(


In this selector you could use composition

export const isAvailableBonusNoneErrorSelector = createSelector( [availableBonusTypesSelector, isAvailableBonusErrorSelector], (abs: AvailableBonusTypesState, hasError: boolean) => hasError && pot.isNone(abs) );

PS you could also leave a little comment for each selector?

ts/features/wallet/component/FeaturedCardCarousel.tsx

…sage

Undermaken · 2021-02-12T09:25:10Z

ts/features/wallet/component/FeaturedCardCarousel.tsx

-  const anyBonusNotActive =
-    (!pot.getOrElse(props.bpdActiveBonus, false) && bpdEnabled) ||
-    (!props.cgnActiveBonus && cgnEnabled);
+  const hasBpdActive: boolean | undefined = pot.getOrElse(


this expression never returns undefined. It returns always a boolean

we should implement this logic

if we have a value (pot some) we can say has bpd or not (true / false, depending of the pot value). Otherwise we can't say nothing (undefined)

Suggested change

const hasBpdActive: boolean | undefined = pot.getOrElse(

const hasBpdActive: boolean | undefined =

props.bpdActiveBonus.kind === "PotSome"

? props.bpdActiveBonus.value

: undefined;

You're absolutely right I thought to commit the previously suggested code:

const hasBpdActive: boolean | undefined = pot.isSome(props.bpdActiveBonus) ? props.bpdActiveBonus.value : undefined;

I'm fixing it asap!

Undermaken · 2021-02-12T09:33:04Z

@CrisTofani since now we are using PR title as changelog detail

what do you think to change PR

from

feat(Carta Giovani Nazionale): [#176825286,#176323469] Clean usage of availableBonusSelector with memoized one #2793

to

feat(Carta Giovani Nazionale): [#176825286,#176323469] Features Carousel: CGN item is shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program (no more while it loading) #2793

CrisTofani · 2021-02-12T09:46:56Z

@CrisTofani since now we are using PR title as changelog detail

what do you think to change PR

from

feat(Carta Giovani Nazionale): [#176825286,#176323469] Clean usage of availableBonusSelector with memoized one #2793

to

feat(Carta Giovani Nazionale): [#176825286,#176323469] Features Carousel: CGN item is shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program (no more while it loading) #2793

@Undermaken maybe

feat(Carta Giovani Nazionale): [#176825286,#176323469] Features Carousel: CGN item is shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

what do you think?

CrisTofani · 2021-02-12T10:14:34Z

@Undermaken a fix from previous proposal:

feat(Carta Giovani Nazionale): [#176825286,#176323469] Features Carousel: Items are now shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

what do you think?

[#176825286] Clean usage of availableBonusSelector with memoized one

e1c0b48

pagopa-github-bot changed the title ~~[#176825286] Clean usage of availableBonusSelector with memoized one~~ feat(Carta Giovani Nazionale): [#176825286] Clean usage of availableBonusSelector with memoized one Feb 8, 2021

CrisTofani marked this pull request as ready for review February 8, 2021 14:31

CrisTofani requested review from debiff, fabriziofff and Undermaken as code owners February 8, 2021 14:31

CrisTofani and others added 2 commits February 10, 2021 11:02

Merge branch 'master' into 176825286-fixes-bonus-available-selector-u…

f3dfdfe

…sage

Merge branch 'master' into 176825286-fixes-bonus-available-selector-u…

3c94f16

…sage

Undermaken suggested changes Feb 11, 2021

View reviewed changes

CrisTofani added 3 commits February 11, 2021 17:15

Merge branch 'master' into 176825286-fixes-bonus-available-selector-u…

21ceb44

…sage

[#176825286] Fix carousel items rendering logic

1200c55

Merge branch 'master' into 176825286-fixes-bonus-available-selector-u…

4acc44b

…sage

CrisTofani requested a review from Undermaken February 11, 2021 18:26

Merge branch 'master' into 176825286-fixes-bonus-available-selector-u…

3e1d3aa

…sage

Undermaken suggested changes Feb 12, 2021

View reviewed changes

[#176825286] add return type

659b6eb

Undermaken changed the title ~~feat(Carta Giovani Nazionale): [#176825286] Clean usage of availableBonusSelector with memoized one~~ feat(Carta Giovani Nazionale): [#176825286,#176323469] Clean usage of availableBonusSelector with memoized one Feb 12, 2021

[#176825286] Fixes check on BpdActive bonus

cf3d311

CrisTofani requested a review from Undermaken February 12, 2021 10:09

pagopa-github-bot changed the title ~~feat(Carta Giovani Nazionale): [#176825286,#176323469] Clean usage of availableBonusSelector with memoized one~~ feat: [#176825286,#176323469] Clean usage of availableBonusSelector with memoized one Feb 12, 2021

[#176825286] add tests

7a8beb0

Undermaken approved these changes Feb 12, 2021

View reviewed changes

Undermaken merged commit 435cde1 into master Feb 12, 2021

Undermaken deleted the 176825286-fixes-bonus-available-selector-usage branch February 12, 2021 11:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [#176825286,#176323469] Features Carousel: Items are now shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

feat: [#176825286,#176323469] Features Carousel: Items are now shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

CrisTofani commented Feb 8, 2021

pagopa-github-bot commented Feb 8, 2021 •

edited

Loading

codecov bot commented Feb 8, 2021 •

edited

Loading

Undermaken Feb 11, 2021

Undermaken Feb 12, 2021

CrisTofani Feb 12, 2021

Undermaken commented Feb 12, 2021

CrisTofani commented Feb 12, 2021

CrisTofani commented Feb 12, 2021

-  const hasBpdActive: boolean | undefined = pot.getOrElse(
+  const hasBpdActive: boolean | undefined =
+    props.bpdActiveBonus.kind === "PotSome"
+      ? props.bpdActiveBonus.value
+      : undefined;

feat: [#176825286,#176323469] Features Carousel: Items are now shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

feat: [#176825286,#176323469] Features Carousel: Items are now shown depending on local and remote feature flag. BPD item is shown only when user is not enrolled to the program, hidden if loading or the information is not available #2793

Conversation

CrisTofani commented Feb 8, 2021

Short description

pagopa-github-bot commented Feb 8, 2021 • edited Loading

Affected stories

codecov bot commented Feb 8, 2021 • edited Loading

Codecov Report

Undermaken Feb 11, 2021

Choose a reason for hiding this comment

Undermaken Feb 12, 2021

Choose a reason for hiding this comment

CrisTofani Feb 12, 2021

Choose a reason for hiding this comment

Undermaken commented Feb 12, 2021

CrisTofani commented Feb 12, 2021

CrisTofani commented Feb 12, 2021

pagopa-github-bot commented Feb 8, 2021 •

edited

Loading

codecov bot commented Feb 8, 2021 •

edited

Loading