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.

Sign up for GitHub

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

Load parent report action from withOnyx in ReportScreen #33970

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Load parent report action from withOnyx in ReportScreen #33970

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3401255
Subscribe to parent report actions in Onyx
tgolen Jan 4, 2024
ce73e90
Get parent report action from props
tgolen Jan 4, 2024
0d7c9db
Merge branch 'main' into tgolen-refactor-reportscreensideloading
tgolen Jan 8, 2024
c897697
Use a selector
tgolen Jan 8, 2024
0cd7315
Merge branch 'main' into tgolen-refactor-reportscreensideloading
tgolen Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions src/pages/home/ReportScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,11 @@ const propTypes = {
/** The report metadata loading states */
reportMetadata: reportMetadataPropTypes,

/** Array of report actions for this report */
reportActions: PropTypes.arrayOf(PropTypes.shape(reportActionPropTypes)),
/** All the report actions for this report */
reportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)),
Copy link
Contributor Author

@tgolen tgolen Jan 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I noticed this was set to an array, but I checked the data in Onyx (it's an object) and the place that uses this data also has it typed in TS as a Record (which would be an object and not an array).


/** The report's parentReportAction */
parentReportAction: PropTypes.shape(reportActionPropTypes),

/** Whether the composer is full size */
isComposerFullSize: PropTypes.bool,
Expand Down Expand Up @@ -103,7 +106,8 @@ const propTypes = {

const defaultProps = {
isSidebarLoaded: false,
reportActions: [],
reportActions: {},
parentReportAction: {},
report: {},
reportMetadata: {
isLoadingInitialReportActions: true,
Expand Down Expand Up @@ -143,6 +147,7 @@ function ReportScreen({
report,
reportMetadata,
reportActions,
parentReportAction,
accountManagerReportID,
personalDetails,
markReadyForHydration,
Expand Down Expand Up @@ -180,18 +185,11 @@ function ReportScreen({

// There are no reportActions at all to display and we are still in the process of loading the next set of actions.
const isLoadingInitialReportActions = _.isEmpty(reportActions) && reportMetadata.isLoadingInitialReportActions;

const isOptimisticDelete = lodashGet(report, 'statusNum') === CONST.REPORT.STATUS.CLOSED;

const shouldHideReport = !ReportUtils.canAccessReport(report, policies, betas);

const isLoading = !reportID || !isSidebarLoaded || _.isEmpty(personalDetails);

const parentReportAction = ReportActionsUtils.getParentReportAction(report);
const isSingleTransactionView = ReportUtils.isMoneyRequest(report);

const policy = policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] || {};

const isTopMostReportId = currentReportID === getReportID(route);
const didSubscribeToReportLeavingEvents = useRef(false);

Expand Down Expand Up @@ -234,7 +232,6 @@ function ReportScreen({
policy={policy}
personalDetails={personalDetails}
isSingleTransactionView={isSingleTransactionView}
parentReportAction={parentReportAction}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out this component no longer references a parentReportAction prop, so I just removed it. The parentReportAction is only used in the MoneyRequestHeader.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! Just in case you were curious, I checked and this type of error would not be possible with TypeScript, so that's good 🙂

/>
);
}
Expand Down Expand Up @@ -569,6 +566,17 @@ export default compose(
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${getReportID(route)}`,
initialValue: false,
},
parentReportAction: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : 0}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a case when parentReportID is empty string?
If not, that's fine but if possible, recommend report && report.parentReportID || 0}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't be possible, no. I checked all of the other places where the parent report actions are fetched using withOnyx and they all use the ternary. It's a little inconsistent whether the ternary returns 0 or the string '0', but since it's a template string that will always cast it all to a string anyway, I guess it doesn't really matter.

selector: (parentReportActions, props) => {
const parentReportActionID = lodashGet(props, 'report.parentReportActionID');
if (!parentReportActionID) {
return {};
}
return parentReportActions[parentReportActionID];
},
canEvict: false,
},
},
true,
),
Expand Down
Loading