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: [IOBP-392] Refactor TimelineOperationListItem component #5200

Merged
merged 12 commits into from
Nov 16, 2023

Conversation

mastro993
Copy link
Contributor

@mastro993 mastro993 commented Nov 6, 2023

Short description

This PR refactors the TimelineOperationListItem component and implements new logic to correctly display CANCELLED and REVERSAL transactions and CIE onboarding operations.

RocketSim_Recording_iPhone_13_2023-11-06_17.47.03.mp4

List of changes proposed in this pull request

How to test

@pagopa-github-bot pagopa-github-bot changed the title [IOBP-392] Refactor TimelineOperationListItem component chore: [IOBP-392] Refactor TimelineOperationListItem component Nov 6, 2023
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Nov 6, 2023

Affected stories

  • ⚙️ IOBP-392: Refactoring TimelineOperationListItem per accomodare nuove operazioni
    subtask of
    • IOBP-276: [B&P] Manutenzione codice

Generated by 🚫 dangerJS against c9dc369

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #5200 (a8211ee) into master (259b651) will increase coverage by 0.26%.
The diff coverage is 90.81%.

❗ Current head a8211ee differs from pull request most recent head c9dc369. Consider uploading reports for the commit c9dc369 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5200      +/-   ##
==========================================
+ Coverage   47.69%   47.96%   +0.26%     
==========================================
  Files        1544     1544              
  Lines       32032    32036       +4     
  Branches     7876     7863      -13     
==========================================
+ Hits        15278    15366      +88     
+ Misses      16704    16620      -84     
  Partials       50       50              
Files Coverage Δ
...details/components/InitiativeTimelineComponent.tsx 79.31% <ø> (ø)
...line/components/TimelineRefundDetailsComponent.tsx 10.00% <ø> (ø)
ts/features/idpay/common/utils/strings.ts 54.54% <75.00%> (+4.54%) ⬆️
...res/idpay/details/screens/OperationsListScreen.tsx 8.82% <0.00%> (ø)
...timeline/components/TimelineDetailsBottomSheet.tsx 44.89% <0.00%> (-0.94%) ⬇️
...y/details/components/TimelineOperationListItem.tsx 93.75% <93.47%> (+85.50%) ⬆️

... and 2 files 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 259b651...c9dc369. Read the comment docs.

@mastro993 mastro993 self-assigned this Nov 6, 2023
@mastro993 mastro993 marked this pull request as ready for review November 6, 2023 16:43
@mastro993 mastro993 requested review from thisisjp and a team as code owners November 6, 2023 16:43
@mastro993 mastro993 added IDPay IO-Bonus e pagamenti IO - Bonus e pagamenti labels Nov 7, 2023
/>
);
};

const TimelineOperationListItemSkeleton = () => (
const SuspendOperationListItem = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that dividing all the listItems like this, rather than just picking the props with the original switch and then using them to render, adds a lot of verbosity for more downsides than benefits; what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you give me some examples?
You mean, instead of React components we should just make regular function to return the props to pass to a ListItemTransaction? Wouldn't you find it difficult to debug and/or mantain instead of just regular React components?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried your suggestion 1578852
I agree with you, I think it's more readable. Let me know!

Copy link
Contributor

@Hantex9 Hantex9 left a comment

Choose a reason for hiding this comment

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

LGTM

@mastro993 mastro993 merged commit 5bc9f2d into master Nov 16, 2023
5 checks passed
@mastro993 mastro993 deleted the IOBP-392-idpay-timeline-refactoring branch November 16, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDPay IO-Bonus e pagamenti IO - Bonus e pagamenti
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants