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

Jump to bottom

[TS migration] Migrate 'SidebarUtils.js' lib to TypeScript #28596

Merged
merged 18 commits into from
Oct 26, 2023
Merged

[TS migration] Migrate 'SidebarUtils.js' lib to TypeScript #28596

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f38e0e5
Initial work
blazejkustra Sep 27, 2023
f42364d
[TS migration] Migrate 'SidebarUtils.js' lib to TypeScript
blazejkustra Oct 2, 2023
08da8c0
Self review the code
blazejkustra Oct 3, 2023
bf4c2f1
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 3, 2023
56abdce
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 12, 2023
1715578
Address code review
blazejkustra Oct 12, 2023
2a25290
Add errors to ReportActionBase
blazejkustra Oct 12, 2023
0e7d40d
Add optional chaining to fix tests
blazejkustra Oct 12, 2023
ccd959e
Fix remaining tests
blazejkustra Oct 12, 2023
24f858d
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 16, 2023
c22c8c9
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 17, 2023
bbb35e8
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 19, 2023
233d703
Bring back test
blazejkustra Oct 20, 2023
f137763
Add comments to onyx types
blazejkustra Oct 20, 2023
47647d5
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 24, 2023
a224678
Remove isLastMessageDeletedParentAction from getOptionData
blazejkustra Oct 24, 2023
b7c5aec
Merge branch 'main' into ts-migration/SidebarUtils
blazejkustra Oct 25, 2023
8d42d7f
Resolve conflicts
blazejkustra Oct 25, 2023
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
4 changes: 2 additions & 2 deletions src/libs/PersonalDetailsUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ Onyx.connect({
});

/**
* @param {Object} passedPersonalDetails
* @param {Array} pathToDisplayName
* @param {Object | Null} passedPersonalDetails
* @param {Array | String} pathToDisplayName
* @param {String} [defaultValue] optional default display name value
* @returns {String}
*/
Expand Down
4 changes: 2 additions & 2 deletions src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ function isConsecutiveActionMadeByPreviousActor(reportActions: ReportAction[] |
/**
* Checks if a reportAction is deprecated.
*/
function isReportActionDeprecated(reportAction: OnyxEntry<ReportAction>, key: string): boolean {
function isReportActionDeprecated(reportAction: OnyxEntry<ReportAction>, key: string | number): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are these string or number now? I'd thought the API should be returning reportActionIDs as strings all the time so if they aren't we need to fix that in the API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here actionKey is number that's why. Which is passed to shouldReportActionBeVisible which invoked isReportActionDeprecated with the same type.

App/src/libs/SidebarUtils.js

Lines 32 to 36 in 1c31bf7

const reportActionsForDisplay = _.filter(
actionsArray,
(reportAction, actionKey) =>
ReportActionsUtils.shouldReportActionBeVisible(reportAction, actionKey) &&
reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED &&

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm think I understand.

so actionKey is an int because in that code for _.filter we're looping over an array which is going to be keyed by an integer?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes index is number!

Copy link
Contributor

Choose a reason for hiding this comment

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

hm okay.

It looks like this is mostly there just for checking if the deprecated sequenceNumbers exist which I think should be fully killed by now but I don't think we should do too much in this refactor so I think it's probably fine to leave as is for now.

but after we merge this we should probably clean up that code, i don't think we need to pass key ever and should be able to only pass reportAction but I'm no the most involved on this side of things either.

if (!reportAction) {
return true;
}
Expand All @@ -280,7 +280,7 @@ function isReportActionDeprecated(reportAction: OnyxEntry<ReportAction>, key: st
* Checks if a reportAction is fit for display, meaning that it's not deprecated, is of a valid
* and supported type, it's not deleted and also not closed.
*/
function shouldReportActionBeVisible(reportAction: OnyxEntry<ReportAction>, key: string): boolean {
function shouldReportActionBeVisible(reportAction: OnyxEntry<ReportAction>, key: string | number): boolean {
if (!reportAction) {
return false;
}
Expand Down
16 changes: 8 additions & 8 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ function getPolicyType(report, policies) {
/**
* Get the policy name from a given report
* @param {Object} report
* @param {String} report.policyID
* @param {String} report.oldPolicyName
* @param {String} report.policyName
* @param {String} [report.policyID]
* @param {String} [report.oldPolicyName]
* @param {String} [report.policyName]
* @param {Boolean} [returnEmptyIfNotFound]
* @param {Object} [policy]
* @returns {String}
Expand Down Expand Up @@ -363,7 +363,7 @@ function isUserCreatedPolicyRoom(report) {
/**
* Whether the provided report is a Policy Expense chat.
* @param {Object} report
* @param {String} report.chatType
* @param {String} [report.chatType]
* @returns {Boolean}
*/
function isPolicyExpenseChat(report) {
Expand All @@ -389,7 +389,7 @@ function isControlPolicyExpenseReport(report) {
/**
* Whether the provided report is a chat room
* @param {Object} report
* @param {String} report.chatType
* @param {String} [report.chatType]
* @returns {Boolean}
*/
function isChatRoom(report) {
Expand Down Expand Up @@ -578,8 +578,8 @@ function findLastAccessedReport(reports, ignoreDomainRooms, policies, isFirstTim
/**
* Whether the provided report is an archived room
* @param {Object} report
* @param {Number} report.stateNum
* @param {Number} report.statusNum
* @param {Number} [report.stateNum]
* @param {Number} [report.statusNum]
* @returns {Boolean}
*/
function isArchivedRoom(report) {
Expand Down Expand Up @@ -3134,7 +3134,7 @@ function shouldHideReport(report, currentReportId) {
* filter out the majority of reports before filtering out very specific minority of reports.
*
* @param {Object} report
* @param {String} currentReportId
* @param {String | Null | Undefined} currentReportId
* @param {Boolean} isInGSDMode
* @param {String[]} betas
* @param {Object} policies
Expand Down
Loading
Loading