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

fix: show disabled distance rate if it is selected in IOU request #47850

Merged
merged 1 commit into from
Sep 9, 2024
Merged

fix: show disabled distance rate if it is selected in IOU request #47850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions src/libs/DistanceRequestUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type MileageRate = {
currency?: string;
unit: Unit;
name?: string;
enabled?: boolean;
};

let lastSelectedDistanceRates: OnyxEntry<LastSelectedDistanceRates> = {};
Expand All @@ -32,7 +33,7 @@ Onyx.connect({
const METERS_TO_KM = 0.001; // 1 kilometer is 1000 meters
const METERS_TO_MILES = 0.000621371; // There are approximately 0.000621371 miles in a meter

function getMileageRates(policy: OnyxInputOrEntry<Policy>, includeDisabledRates = false): Record<string, MileageRate> {
function getMileageRates(policy: OnyxInputOrEntry<Policy>, includeDisabledRates = false, selectedRateID?: string): Record<string, MileageRate> {
const mileageRates: Record<string, MileageRate> = {};

if (!policy?.customUnits) {
Expand All @@ -45,7 +46,7 @@ function getMileageRates(policy: OnyxInputOrEntry<Policy>, includeDisabledRates
}

Object.entries(distanceUnit.rates).forEach(([rateID, rate]) => {
if (!includeDisabledRates && rate.enabled === false) {
if (!includeDisabledRates && rate.enabled === false && (!selectedRateID || rateID !== selectedRateID)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

!selectedRateID would fail if we have a rateID that's "0" no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If selectedRateID = "0" this condition !selectedRateID will be false (which is expected), no?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, "0" would be the ID, which means it has an ID

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, "0" would be the ID, which means it has an ID

Sorry, I don't quite follow what you mean by '0' being the ID. Could you clarify that for me?

Copy link
Contributor

Choose a reason for hiding this comment

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

If I create a rate whose ID is the string "0" then select it, this would not work because !"0" is false but that's wrong because we indeed passed an ID, so we should've compared selectedRateID with rateID but we did not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so we should've compared selectedRateID with rateID but we did not

  • No. I think with the current condition:
if (!includeDisabledRates && rate.enabled === false && (!selectedRateID || rateID !== selectedRateID)) {

we still compare selectedRateID with rateID via rateID !== selectedRateID.

CMIIWW. Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you are right! I was confusing the behavior with PHP where "0" is falsy

return;
}

Expand All @@ -55,6 +56,7 @@ function getMileageRates(policy: OnyxInputOrEntry<Policy>, includeDisabledRates
unit: distanceUnit.attributes.unit,
name: rate.name,
customUnitRateID: rate.customUnitRateID,
enabled: rate.enabled,
};
});

Expand Down
4 changes: 3 additions & 1 deletion src/pages/iou/request/step/IOURequestStepDistanceRate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ function IOURequestStepDistanceRate({
const [policyDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${IOU.getIOURequestPolicyID(transaction, reportDraft) ?? '-1'}`);

const policy = policyReal ?? policyDraft;
const rates = DistanceRequestUtils.getMileageRates(policy);

const styles = useThemeStyles();
const {translate, toLocaleDigit} = useLocalize();
Expand All @@ -65,6 +64,8 @@ function IOURequestStepDistanceRate({

const currentRateID = TransactionUtils.getRateID(transaction) ?? '-1';

const rates = DistanceRequestUtils.getMileageRates(policy, false, currentRateID);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see the change to remove the Onyx keys. Is it already removed in another PR/refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is removed in #46859


const navigateBack = () => {
Navigation.goBack(backTo);
};
Expand All @@ -77,6 +78,7 @@ function IOURequestStepDistanceRate({
alternateText: rate.name ? rateForDisplay : '',
keyForList: rate.customUnitRateID,
value: rate.customUnitRateID,
isDisabled: !rate.enabled,
isSelected: currentRateID ? currentRateID === rate.customUnitRateID : rate.name === CONST.CUSTOM_UNITS.DEFAULT_RATE,
};
});
Expand Down
Loading