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(Carta Giovani Nazionale): [#176958889] Implementation of the redux store/actions section for Merchants list load and storage #2843

Merged
merged 173 commits into from
Mar 4, 2021

Conversation

CrisTofani
Copy link
Contributor

@CrisTofani CrisTofani commented Feb 23, 2021

This PR Depens on #2807

Short description

This PR adds the scaffolding implementation of the store and reducer for CGN merchants list

CrisTofani and others added 30 commits January 12, 2021 17:53
fabriziofff and others added 23 commits February 24, 2021 11:53
Co-authored-by: fabriziofff <[email protected]>
…m:pagopa/io-app into 176958889-store-reducer-cgn-merchants
import { cgnMerchants } from "../actions/merchants";

export type CgnMerchantsState = {
list: pot.Pot<ReadonlyArray<any>, NetworkError>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
list: pot.Pot<ReadonlyArray<any>, NetworkError>;
merchants: pot.Pot<ReadonlyArray<any>, NetworkError>;

Could it be a more descriptive name?
tip: if this store section includes only merchants, it can be directly the merchants list
export type CgnMerchantsState = pot.Pot<ReadonlyArray<any>, NetworkError>;

Copy link
Contributor Author

@CrisTofani CrisTofani Mar 4, 2021

Choose a reason for hiding this comment

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

@Undermaken
I used merchants as the attribute name in the CgnState since this section should have inside the merchants list and the selected merchant to show it's detail avoiding a navigation parameter. This was how i figured out the store:

{
  ...globalState,
  cgn: {
    ..cgnState,
    merchants: {
      list: [],
      selectedMerchant: {}
    }
  }
}

what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good for me if this section will store also other data

"CGN_MERCHANTS_REQUEST",
"CGN_MERCHANTS_SUCCESS",
"CGN_MERCHANTS_FAILURE"
)<void, ReadonlyArray<any>, NetworkError>();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid to use any
I guess it is just a placeholder but if we forget to replace it we can use the action and the reducer with no warnings from compiler.

Could instead possibile to create and use a temporary type?
type CGNMerchant = unknown;

@CrisTofani CrisTofani requested a review from Undermaken March 4, 2021 15:42
@Undermaken Undermaken merged commit 0ec802b into master Mar 4, 2021
@Undermaken Undermaken deleted the 176958889-store-reducer-cgn-merchants branch March 4, 2021 17:46
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.

5 participants