Skip to content

Commit d1c56c9

Browse files
chore(runway): cherry-pick fix: invalid order book buttons cp-7.62.0 (#24889)
- fix: invalid order book buttons cp-7.62.0 (#24878) ## **Description** This PR fixes the action buttons on the Order Book screen (`PerpsOrderBookView`) to show **Modify/Close** buttons when the user has an open position, instead of always showing **Long/Short** buttons. **Problem:** The Order Book screen always displayed Long/Short buttons, even when the user had an existing position for that market. **Solution:** Added position-checking logic (consistent with `PerpsMarketDetailsView`) to conditionally render: - **Modify/Close** buttons when user has an existing position - **Long/Short** buttons when no position exists ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: [TAT-2224](https://consensyssoftware.atlassian.net/browse/TAT-2224) #24879 ## **Manual testing steps** ```gherkin Feature: Order Book action buttons based on position Scenario: User views Order Book without an open position Given user has no open position for BTC When user navigates to the BTC Order Book screen Then user sees "Long" and "Short" buttons Scenario: User views Order Book with an open long position Given user has an open long position for BTC When user navigates to the BTC Order Book screen Then user sees "Modify" and "Close Long" buttons Scenario: User views Order Book with an open short position Given user has an open short position for BTC When user navigates to the BTC Order Book screen Then user sees "Modify" and "Close Short" buttons Scenario: User taps Modify button Given user has an open position for BTC And user is on the BTC Order Book screen When user taps the "Modify" button Then the modify action sheet opens Scenario: User taps Close button Given user has an open position for BTC And user is on the BTC Order Book screen When user taps the "Close Long" or "Close Short" button Then user is navigated to the close position flow ``` ## **Screenshots/Recordings** ### **Before** - Order Book always shows Long/Short buttons regardless of position status ### **After** - Order Book shows Modify/Close buttons when position exists - Order Book shows Long/Short buttons when no position exists https://github.com/user-attachments/assets/c3427894-b62b-42ab-b5a2-d49157c2bc38 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. [TAT-2224]: https://consensyssoftware.atlassian.net/browse/TAT-2224?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Updates `PerpsOrderBookView` to reflect existing positions and enable position management actions. > > - Conditionally render `Modify` and `Close` buttons (replacing `Long`/`Short`) when `useHasExistingPosition` returns a position; label `Close Long`/`Close Short` based on size sign > - Wire actions: `Modify` opens bottom sheet via `usePositionManagement`; `Close` navigates with `navigateToClosePosition`; render `PerpsSelectModifyActionView` when sheet is visible > - Preserve `Long`/`Short` buttons when no position exists; maintain A/B color handling > - Add new test IDs (`perps-order-book-modify-button`, `perps-order-book-close-button`) and i18n strings > - Expand tests to cover conditional rendering, modify sheet open, and close navigation > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit be0ca08. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [8293e94](8293e94) [TAT-2224]: https://consensyssoftware.atlassian.net/browse/TAT-2224?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [TAT-2224]: https://consensyssoftware.atlassian.net/browse/TAT-2224?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: abretonc7s <107169956+abretonc7s@users.noreply.github.com>
1 parent 5168c3f commit d1c56c9

3 files changed

Lines changed: 309 additions & 40 deletions

File tree

app/components/UI/Perps/Perps.testIds.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,8 @@ export const PerpsOrderBookViewSelectorsIDs = {
635635
TABLE: 'perps-order-book-table',
636636
LONG_BUTTON: 'perps-order-book-long-button',
637637
SHORT_BUTTON: 'perps-order-book-short-button',
638+
MODIFY_BUTTON: 'perps-order-book-modify-button',
639+
CLOSE_BUTTON: 'perps-order-book-close-button',
638640
DEPTH_BAND_BUTTON: 'perps-order-book-depth-band-button',
639641
DEPTH_BAND_OPTION: 'perps-order-book-depth-band-option',
640642
UNIT_TOGGLE_BASE: 'perps-order-book-unit-toggle-base',

app/components/UI/Perps/Views/PerpsOrderBookView/PerpsOrderBookView.test.tsx

Lines changed: 195 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ jest.mock('../../../../../../locales/i18n', () => ({
3737
'perps.order_book.error': 'Failed to load order book',
3838
'perps.market.long': 'Long',
3939
'perps.market.short': 'Short',
40+
'perps.market.modify': 'Modify',
41+
'perps.market.close_long': 'Close Long',
42+
'perps.market.close_short': 'Close Short',
4043
'perps.order_book.depth_band.title': 'Depth Band',
4144
};
4245
return translations[key] || key;
@@ -121,12 +124,18 @@ jest.mock('../../hooks/usePerpsMeasurement', () => ({
121124
usePerpsMeasurement: jest.fn(),
122125
}));
123126

124-
// Mock usePerpsNavigation and usePerpsMarkets
127+
// Mock usePerpsNavigation, usePerpsMarkets, and usePositionManagement
125128
const mockNavigateToOrder = jest.fn();
129+
const mockNavigateToClosePosition = jest.fn();
130+
const mockOpenModifySheet = jest.fn();
131+
const mockCloseModifySheet = jest.fn();
132+
const mockHandleReversePosition = jest.fn();
133+
const mockModifyActionSheetRef = { current: null };
126134

127135
jest.mock('../../hooks', () => ({
128136
usePerpsNavigation: jest.fn(() => ({
129137
navigateToOrder: mockNavigateToOrder,
138+
navigateToClosePosition: mockNavigateToClosePosition,
130139
})),
131140
usePerpsMarkets: jest.fn(() => ({
132141
markets: [
@@ -139,6 +148,21 @@ jest.mock('../../hooks', () => ({
139148
isLoading: false,
140149
error: null,
141150
})),
151+
usePositionManagement: jest.fn(() => ({
152+
showModifyActionSheet: false,
153+
modifyActionSheetRef: mockModifyActionSheetRef,
154+
openModifySheet: mockOpenModifySheet,
155+
closeModifySheet: mockCloseModifySheet,
156+
handleReversePosition: mockHandleReversePosition,
157+
})),
158+
}));
159+
160+
// Mock useHasExistingPosition
161+
jest.mock('../../hooks/useHasExistingPosition', () => ({
162+
useHasExistingPosition: jest.fn(() => ({
163+
isLoading: false,
164+
existingPosition: null,
165+
})),
142166
}));
143167

144168
// Mock usePerpsOrderBookGrouping
@@ -238,6 +262,15 @@ jest.mock(
238262
},
239263
);
240264

265+
// Mock PerpsSelectModifyActionView
266+
jest.mock('../PerpsSelectModifyActionView', () => {
267+
const { View } = jest.requireActual('react-native');
268+
return {
269+
__esModule: true,
270+
default: () => <View testID="perps-select-modify-action-view" />,
271+
};
272+
});
273+
241274
describe('PerpsOrderBookView', () => {
242275
const initialState = {
243276
engine: {
@@ -576,6 +609,167 @@ describe('PerpsOrderBookView', () => {
576609
});
577610
});
578611

612+
describe('action buttons with existing position', () => {
613+
const mockLongPosition = {
614+
coin: 'BTC',
615+
size: '1.5',
616+
entryPrice: '50000',
617+
leverage: { value: 10, type: 'cross' as const },
618+
margin: '5000',
619+
unrealizedPnl: '100',
620+
unrealizedPnlPercent: '2',
621+
liquidationPrice: '45000',
622+
takeProfitPrice: undefined,
623+
stopLossPrice: undefined,
624+
returnOnEquity: '2',
625+
};
626+
627+
const mockShortPosition = {
628+
...mockLongPosition,
629+
size: '-1.5',
630+
};
631+
632+
beforeEach(() => {
633+
jest.clearAllMocks();
634+
});
635+
636+
it('renders Modify and Close buttons when position exists', () => {
637+
const { useHasExistingPosition } = jest.requireMock(
638+
'../../hooks/useHasExistingPosition',
639+
);
640+
useHasExistingPosition.mockReturnValue({
641+
isLoading: false,
642+
existingPosition: mockLongPosition,
643+
});
644+
645+
const { getByTestId, queryByTestId } = renderWithProvider(
646+
<PerpsOrderBookView />,
647+
{
648+
state: initialState,
649+
},
650+
);
651+
652+
expect(
653+
getByTestId(PerpsOrderBookViewSelectorsIDs.MODIFY_BUTTON),
654+
).toBeOnTheScreen();
655+
expect(
656+
getByTestId(PerpsOrderBookViewSelectorsIDs.CLOSE_BUTTON),
657+
).toBeOnTheScreen();
658+
expect(
659+
queryByTestId(PerpsOrderBookViewSelectorsIDs.LONG_BUTTON),
660+
).toBeNull();
661+
expect(
662+
queryByTestId(PerpsOrderBookViewSelectorsIDs.SHORT_BUTTON),
663+
).toBeNull();
664+
});
665+
666+
it('opens modify action sheet when Modify button is pressed', () => {
667+
const { useHasExistingPosition } = jest.requireMock(
668+
'../../hooks/useHasExistingPosition',
669+
);
670+
useHasExistingPosition.mockReturnValue({
671+
isLoading: false,
672+
existingPosition: mockLongPosition,
673+
});
674+
675+
const { getByTestId } = renderWithProvider(<PerpsOrderBookView />, {
676+
state: initialState,
677+
});
678+
679+
const modifyButton = getByTestId(
680+
PerpsOrderBookViewSelectorsIDs.MODIFY_BUTTON,
681+
);
682+
fireEvent.press(modifyButton);
683+
684+
expect(mockOpenModifySheet).toHaveBeenCalled();
685+
});
686+
687+
it('navigates to close position when Close button is pressed', () => {
688+
const { useHasExistingPosition } = jest.requireMock(
689+
'../../hooks/useHasExistingPosition',
690+
);
691+
useHasExistingPosition.mockReturnValue({
692+
isLoading: false,
693+
existingPosition: mockLongPosition,
694+
});
695+
696+
const { getByTestId } = renderWithProvider(<PerpsOrderBookView />, {
697+
state: initialState,
698+
});
699+
700+
const closeButton = getByTestId(
701+
PerpsOrderBookViewSelectorsIDs.CLOSE_BUTTON,
702+
);
703+
fireEvent.press(closeButton);
704+
705+
expect(mockNavigateToClosePosition).toHaveBeenCalledWith(
706+
mockLongPosition,
707+
);
708+
});
709+
710+
it('displays "Close Long" for long positions', () => {
711+
const { useHasExistingPosition } = jest.requireMock(
712+
'../../hooks/useHasExistingPosition',
713+
);
714+
useHasExistingPosition.mockReturnValue({
715+
isLoading: false,
716+
existingPosition: mockLongPosition,
717+
});
718+
719+
const { getByText } = renderWithProvider(<PerpsOrderBookView />, {
720+
state: initialState,
721+
});
722+
723+
expect(getByText('Close Long')).toBeOnTheScreen();
724+
});
725+
726+
it('displays "Close Short" for short positions', () => {
727+
const { useHasExistingPosition } = jest.requireMock(
728+
'../../hooks/useHasExistingPosition',
729+
);
730+
useHasExistingPosition.mockReturnValue({
731+
isLoading: false,
732+
existingPosition: mockShortPosition,
733+
});
734+
735+
const { getByText } = renderWithProvider(<PerpsOrderBookView />, {
736+
state: initialState,
737+
});
738+
739+
expect(getByText('Close Short')).toBeOnTheScreen();
740+
});
741+
742+
it('shows Long/Short buttons when no position exists', () => {
743+
const { useHasExistingPosition } = jest.requireMock(
744+
'../../hooks/useHasExistingPosition',
745+
);
746+
useHasExistingPosition.mockReturnValue({
747+
isLoading: false,
748+
existingPosition: null,
749+
});
750+
751+
const { getByTestId, queryByTestId } = renderWithProvider(
752+
<PerpsOrderBookView />,
753+
{
754+
state: initialState,
755+
},
756+
);
757+
758+
expect(
759+
getByTestId(PerpsOrderBookViewSelectorsIDs.LONG_BUTTON),
760+
).toBeOnTheScreen();
761+
expect(
762+
getByTestId(PerpsOrderBookViewSelectorsIDs.SHORT_BUTTON),
763+
).toBeOnTheScreen();
764+
expect(
765+
queryByTestId(PerpsOrderBookViewSelectorsIDs.MODIFY_BUTTON),
766+
).toBeNull();
767+
expect(
768+
queryByTestId(PerpsOrderBookViewSelectorsIDs.CLOSE_BUTTON),
769+
).toBeNull();
770+
});
771+
});
772+
579773
describe('error state', () => {
580774
it('displays error message when order book fails to load', () => {
581775
mockUsePerpsLiveOrderBook.mockReturnValue({

0 commit comments

Comments
 (0)