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

Pass currency and do not pass transaction_ids parameter to create instant deposit #7828

Merged
merged 6 commits into from
Jan 15, 2024
Merged

Pass currency and do not pass transaction_ids parameter to create instant deposit #7828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abe9398
Pass currency and do not pass transaction_ids parameter to create ins…
shendy-a8c Dec 4, 2023
a90c078
Changelog.
shendy-a8c Dec 4, 2023
4ff88a6
Merge branch 'develop' into add/7711-instant-deposit-currency
shendy-a8c Dec 4, 2023
2571606
Merge branch 'develop' into add/7711-instant-deposit-currency
shendy-a8c Dec 10, 2023
08ec968
Merge branch 'develop' into add/7711-instant-deposit-currency
shendy-a8c Jan 9, 2024
c8a6a92
Merge branch 'develop' into add/7711-instant-deposit-currency
shendy-a8c Jan 15, 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
4 changes: 4 additions & 0 deletions changelog/add-7711-instant-deposit-currency
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: update

Pass currency parameter and not transaction_ids parameter when creating instant deposit.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we add a changelog entry for this kind of change, one that is not user facing and a change that is only 'in the background'?

I assume yes but like a confirmation.

1 change: 0 additions & 1 deletion client/components/account-balances/test/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ const createMockOverview = (
fee: 0,
net: 0,
fee_percentage: 0,
transaction_ids: [],
},
};
};
Expand Down
1 change: 0 additions & 1 deletion client/components/deposits-overview/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ const createMockOverview = (
fee: 0,
net: 0,
fee_percentage: 0,
transaction_ids: [],
},
};
};
Expand Down
8 changes: 4 additions & 4 deletions client/data/deposits/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,18 @@ export function updateInstantDeposit( data ) {
};
}

export function* submitInstantDeposit( transactionIds ) {
export function* submitInstantDeposit( currency ) {
try {
yield dispatch( STORE_NAME ).startResolution( 'getInstantDeposit', [
transactionIds,
currency,
] );

const deposit = yield apiFetch( {
path: '/wc/v3/payments/deposits',
method: 'POST',
data: {
type: 'instant',
transaction_ids: transactionIds,
currency,
},
} );

Expand Down Expand Up @@ -153,7 +153,7 @@ export function* submitInstantDeposit( transactionIds ) {
);
} finally {
yield dispatch( STORE_NAME ).finishResolution( 'getInstantDeposit', [
transactionIds,
currency,
] );
}
}
8 changes: 4 additions & 4 deletions client/data/deposits/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,18 +235,18 @@ export const useDepositsSummary = ( {
};

export const useInstantDeposit = (
transactionIds: string[]
currency: string
): { inProgress: boolean; submit: () => void; deposit: unknown } => {
Copy link
Member

Choose a reason for hiding this comment

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

Thought (out of scope of this PR): we should use an interface from deposits.d.ts here.

However, low priority, since this value from this hook is actually not currently used in our codebase (we only use inProgress and submit).

Copy link
Contributor Author

@shendy-a8c shendy-a8c Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was wondering about that actually, whether it's necessary to return deposit since it's not used anywhere.

Do you think I should delete deposit, inProgress from the return value, and getInstantDeposit?

I tried to do that and there are a lot of things can be deleted. However, I thought they are necessary to keep the state of the button so we can use it to remove the button after an instant deposit is created but actually, even after preserving them (deposit and inProgress), the button still does not disappear after an instant deposit is created. I created #7841 for that.

const { deposit, inProgress } = useSelect( ( select ) => {
const { getInstantDeposit, isResolving } = select( STORE_NAME );

return {
deposit: getInstantDeposit( [ transactionIds ] ),
inProgress: isResolving( 'getInstantDeposit', [ transactionIds ] ),
deposit: getInstantDeposit( [ currency ] ),
inProgress: isResolving( 'getInstantDeposit', [ currency ] ),
};
} );
const { submitInstantDeposit } = useDispatch( STORE_NAME );
const submit = () => submitInstantDeposit( transactionIds );
const submit = () => submitInstantDeposit( currency );

return { deposit, inProgress, submit };
};
3 changes: 1 addition & 2 deletions client/data/deposits/test/overviews.fixture.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@
"amount": 12345,
"fee": 185.175,
"net": 12159.83,
"fee_percentage": 1.5,
"transaction_ids": [ "txn_XYZ", "txn_ZXY" ]
"fee_percentage": 1.5
}
]
},
Expand Down
4 changes: 1 addition & 3 deletions client/deposits/instant-deposits/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ const InstantDepositButton: React.FC< InstantDepositButtonProps > = ( {
} ) => {
const [ isModalOpen, setModalOpen ] = useState( false );
const buttonDisabled = isButtonDisabled( instantBalance );
const { inProgress, submit } = useInstantDeposit(
instantBalance.transaction_ids
);
const { inProgress, submit } = useInstantDeposit( instantBalance.currency );
const onClose = () => {
setModalOpen( false );
};
Expand Down
2 changes: 0 additions & 2 deletions client/deposits/instant-deposits/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const mockInstantBalance = {
fee: 123.45,
net: 12221.55,
fee_percentage: 1.5,
transaction_ids: [ 'txn_ABC123', 'txn_DEF456' ],
currency: 'USD',
} as AccountOverview.InstantBalance;

Expand All @@ -37,7 +36,6 @@ const mockZeroInstantBalance = {
fee: 0,
net: 0,
fee_percentage: 1.5,
transaction_ids: [],
currency: 'USD',
} as AccountOverview.InstantBalance;

Expand Down
1 change: 0 additions & 1 deletion client/types/account-overview.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export interface InstantBalance {
fee: number;
net: number;
fee_percentage: number;
transaction_ids: Array< string >;
Copy link
Member

Choose a reason for hiding this comment

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

👍

}

export interface Overview {
Expand Down
2 changes: 1 addition & 1 deletion includes/admin/class-wc-rest-payments-deposits-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,6 @@ static function ( $filter ) {
*/
public function manual_deposit( $request ) {
$params = $request->get_params();
return $this->forward_request( 'manual_deposit', [ $params['type'], $params['transaction_ids'] ] );
return $this->forward_request( 'manual_deposit', [ $params['type'], $params['currency'] ] );
}
}
8 changes: 4 additions & 4 deletions includes/wc-payment-api/class-wc-payments-api-client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,15 @@ public function get_deposits_summary( array $filters = [] ) {
* Trigger a manual deposit.
*
* @param string $type Type of deposit. Only "instant" is supported for now.
* @param string $transaction_ids Comma-separated list of transaction IDs that will be associated with this deposit.
* @param string $currency The deposit currency.
* @return array The new deposit object.
* @throws API_Exception - Exception thrown on request failure.
*/
public function manual_deposit( $type, $transaction_ids ) {
public function manual_deposit( $type, $currency ) {
return $this->request(
[
'type' => $type,
'transaction_ids' => $transaction_ids,
'type' => $type,
'currency' => $currency,
],
self::DEPOSITS_API,
self::POST
Expand Down
Loading