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

Remove Report Fields Beta #44695

Merged
merged 5 commits into from
Jul 12, 2024
+3 −17
Merged

Remove Report Fields Beta #44695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d0a59fd
Remove REPORT_FIELDS beta
neonbhai Jun 30, 2024
a14654a
Merge branch 'Expensify:main' into remove-beta-report-fields
neonbhai Jul 1, 2024
aa56076
Merge branch 'Expensify:main' into remove-beta-report-fields
neonbhai Jul 8, 2024
fa20e0f
Merge branch 'Expensify:main' into remove-beta-report-fields
neonbhai Jul 11, 2024
35045fc
Remove unused const
neonbhai Jul 11, 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
1 change: 0 additions & 1 deletion src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,6 @@ const CONST = {
DEFAULT_ROOMS: 'defaultRooms',
VIOLATIONS: 'violations',
DUPE_DETECTION: 'dupeDetection',
REPORT_FIELDS: 'reportFields',
P2P_DISTANCE_REQUESTS: 'p2pDistanceRequests',
WORKFLOWS_DELAYED_SUBMISSION: 'workflowsDelayedSubmission',
SPOTNANA_TRAVEL: 'spotnanaTravel',
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyReportView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function MoneyReportView({report, policy}: MoneyReportViewProps) {
<AnimatedEmptyStateBackground />
{!ReportUtils.isClosedExpenseReportWithNoExpenses(report) && (
<>
{ReportUtils.reportFieldsEnabled(report) &&
{ReportUtils.isPaidGroupPolicyExpenseReport(report) &&
sortedPolicyReportFields.map((reportField) => {
if (ReportUtils.isReportFieldOfTypeTitle(reportField)) {
return null;
Expand Down
5 changes: 0 additions & 5 deletions src/libs/Permissions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ function canUseDefaultRooms(betas: OnyxEntry<Beta[]>): boolean {
return !!betas?.includes(CONST.BETAS.DEFAULT_ROOMS) || canUseAllBetas(betas);
}

function canUseReportFields(betas: OnyxEntry<Beta[]>): boolean {
return !!betas?.includes(CONST.BETAS.REPORT_FIELDS) || canUseAllBetas(betas);
}

function canUseViolations(betas: OnyxEntry<Beta[]>): boolean {
return !!betas?.includes(CONST.BETAS.VIOLATIONS) || canUseAllBetas(betas);
}
Expand Down Expand Up @@ -73,7 +69,6 @@ export default {
canUseLinkPreviews,
canUseViolations,
canUseDupeDetection,
canUseReportFields,
canUseP2PDistanceRequests,
canUseWorkflowsDelayedSubmission,
canUseSpotnanaTravel,
Expand Down
12 changes: 2 additions & 10 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2478,13 +2478,6 @@ function isHoldCreator(transaction: OnyxEntry<Transaction>, reportID: string): b
return isActionCreator(holdReportAction);
}

/**
* Check if report fields are available to use in a report
*/
function reportFieldsEnabled(report: Report) {
return Permissions.canUseReportFields(allBetas ?? []) && isPaidGroupPolicyExpenseReport(report);
}

/**
* Given a report field, check if the field can be edited or not.
* For title fields, its considered disabled if `deletable` prop is `true` (https://github.com/Expensify/App/issues/35043#issuecomment-1911275433)
Expand Down Expand Up @@ -2586,7 +2579,7 @@ function getMoneyRequestReportName(report: OnyxEntry<Report>, policy?: OnyxEntry
const reportFields = isReportSettled ? report?.fieldList : getReportFieldsByPolicyID(report?.policyID ?? '-1');
const titleReportField = getFormulaTypeReportField(reportFields ?? {});

if (titleReportField && report?.reportName && reportFieldsEnabled(report)) {
if (titleReportField && report?.reportName && isPaidGroupPolicyExpenseReport(report)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These checks need to be updated from isPaidGroupPolicyExpenseReport to isControlPolicyExpenseReport

Report fields are going to be a control only feature (unless it's the title field)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thienlnam hi, this logic seems to actually check for the title field and return the report name according to formula. So we probably do want this as it is right now?

Same for this line which is creating an optimistic report and getting the title field.

The check on this line and is for showing the Report Fields in the MoneyReportView. Changing this to isControlPolicyExpenseReport means we would hide the title field, since the render logic is coupled with report fields. I could fix it if you suggest (We would change the getAvailableReportFields to two functions, one which returns the title, other handling remaining fields)

If we change the check, it might flag as a regression since Report Fields are still configurable on OldDot for Collect Policies:

Video
Screen.Recording.2024-07-12.at.9.01.23.AM.mov

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yeah that's a fair point about how they are coupled right now. In that case, we probably don't need to do anything right now - we'll handle that later when we remove it from collect polcies

return report.reportName;
}

Expand Down Expand Up @@ -3995,7 +3988,7 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa
}

const titleReportField = getTitleReportField(getReportFieldsByPolicyID(policyID) ?? {});
if (!!titleReportField && reportFieldsEnabled(expenseReport)) {
if (!!titleReportField && isPaidGroupPolicyExpenseReport(expenseReport)) {
expenseReport.reportName = populateOptimisticReportFormula(titleReportField.defaultValue, expenseReport, policy);
}

Expand Down Expand Up @@ -7338,7 +7331,6 @@ export {
navigateBackAfterDeleteTransaction,
parseReportRouteParams,
parseReportActionHtmlToText,
reportFieldsEnabled,
requiresAttentionFromCurrentUser,
shouldAutoFocusOnKeyPress,
shouldCreateNewMoneyRequestReport,
Expand Down
Loading