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

Ameerul /Bug 74362 Give feedback button does nothing when user in existing order page #6456

Merged
merged 15 commits into from
Oct 19, 2022
Merged

Ameerul /Bug 74362 Give feedback button does nothing when user in existing order page #6456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4b3a061
added new function in notifications store to get order id and show ra…
ameerul-deriv Sep 14, 2022
ea78d3c
fixed spelling
ameerul-deriv Sep 14, 2022
4d569fd
renamed order_props to p2p_order_props and added toggleNotificationsM…
ameerul-deriv Sep 15, 2022
d57ed78
fixed give action onClick for give feedback button in general store
ameerul-deriv Sep 15, 2022
aaeac4a
Merge branch 'master' of github.com:binary-com/deriv-app into bug-743…
ameerul-deriv Sep 15, 2022
6f76f3e
Merge branch 'master' of github.com:binary-com/deriv-app into bug-743…
ameerul-deriv Sep 20, 2022
40a5f26
added new function to redirect to order details page, changed logic f…
ameerul-deriv Sep 21, 2022
7a23953
removed text from Give Feedback
ameerul-deriv Sep 21, 2022
59dbeb2
fixed notifications count bell bug when rating order, and refactored …
ameerul-deriv Sep 21, 2022
845562c
Merge branch 'master' of github.com:binary-com/deriv-app into bug-743…
ameerul-deriv Sep 21, 2022
a07a152
removed setP2POrderConfig from order-details, moved to app.jsx in use…
ameerul-deriv Sep 21, 2022
5bcd9a2
called setP2POrderProps in order-details when unmounted so the id is …
ameerul-deriv Sep 21, 2022
dc331bf
renamed notification_action to notification_redirect_action
ameerul-deriv Sep 21, 2022
ea704f6
removed commented out console log
ameerul-deriv Sep 21, 2022
27ef4e0
reverted commit and fixed conflicts
ameerul-deriv Oct 17, 2022
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 packages/cashier/src/pages/p2p-cashier/p2p-cashier.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const P2PCashier = ({
removeNotificationByKey,
removeNotificationMessage,
residence,
setP2POrderProps,
setNotificationCount,
setCurrentFocus,
balance,
Expand Down Expand Up @@ -140,6 +141,7 @@ const P2PCashier = ({
setNotificationCount={setNotificationCount}
setOrderId={setQueryOrder}
setOnRemount={setOnRemount}
setP2POrderProps={setP2POrderProps}
should_show_verification={/verification/.test(location.hash)}
verification_action={action_param}
verification_code={code_param}
Expand Down Expand Up @@ -169,6 +171,7 @@ P2PCashier.propTypes = {
residence: PropTypes.string,
setNotificationCount: PropTypes.func,
setCurrentFocus: PropTypes.func,
setP2POrderProps: PropTypes.func,
};

export default connect(({ client, common, modules, notifications, ui }) => ({
Expand All @@ -189,6 +192,7 @@ export default connect(({ client, common, modules, notifications, ui }) => ({
residence: client.residence,
setNotificationCount: modules.cashier.general_store.setNotificationCount,
setOnRemount: modules.cashier.general_store.setOnRemount,
setP2POrderProps: notifications.setP2POrderProps,
is_mobile: ui.is_mobile,
setCurrentFocus: ui.setCurrentFocus,
current_focus: ui.current_focus,
Expand Down
42 changes: 38 additions & 4 deletions packages/core/src/Stores/notification-store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export default class NotificationStore extends BaseStore {
@observable push_notifications = [];
@observable client_notifications = {};
@observable should_show_popups = true;
@observable p2p_order_props = {};

constructor(root_store) {
super({ root_store });
Expand All @@ -55,6 +56,7 @@ export default class NotificationStore extends BaseStore {
root_store.common?.selected_contract_type,
root_store.client.is_eu,
root_store.client.has_enabled_two_fa,
this.p2p_order_props.order_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.

added p2p_order_props.order_id in reactions as notifications count would not be updated when the user has completed rating an order and then navigates out of the order-details page. Even though the order_id has been reset to null when unmounting order_details, the p2p_order_props.order_id in showCompletedOrderNotification function doesn't recognise that p2p_order_props has been updated and won't refresh the notifications. Hence I needed to check it here and refresh the notifications here.

],
async () => {
if (
Expand Down Expand Up @@ -435,14 +437,41 @@ export default class NotificationStore extends BaseStore {
}
}

@action.bound
showCompletedOrderNotification(advertiser_name, order_id) {
const notification_key = `order-${order_id}`;

const notification_redirect_action =
routes.cashier_p2p === window.location.pathname
? {
onClick: () => {
this.p2p_order_props.redirectToOrderDetails(order_id);
this.setP2POrderProps({
...this.p2p_order_props,
order_id,
});
if (this.is_notifications_visible) this.toggleNotificationsModal();
this.refreshNotifications();
},
text: localize('Give feedback'),
}
: {
route: `${routes.cashier_p2p}?order=${order_id}`,
text: localize('Give feedback'),
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the user is within p2p, if they click give feedback, it doesn't navigate them to the order details page of that order. Hence I needed to take the redirectToOrderDetails function from p2p to redirect them to that page in p2p.


this.addNotificationMessage({
action: {
route: `${routes.cashier_p2p}?order=${order_id}`,
text: localize('Give feedback'),
},
action:
this.p2p_order_props?.order_id === order_id
? {
onClick: () => {
this.p2p_order_props.setIsRatingModalOpen(true);
if (this.is_notifications_visible) this.toggleNotificationsModal();
this.refreshNotifications();
},
text: localize('Give feedback'),
}
: notification_redirect_action,
header: <Localize i18n_default_text='Your order {{order_id}} is complete' values={{ order_id }} />,
key: notification_key,
message: (
Expand Down Expand Up @@ -1114,6 +1143,11 @@ export default class NotificationStore extends BaseStore {
this.client_notifications = notifications;
}

@action.bound
setP2POrderProps(p2p_order_props) {
this.p2p_order_props = p2p_order_props;
}

@action.bound
setShouldShowPopups(should_show_popups) {
this.should_show_popups = should_show_popups;
Expand Down
2 changes: 1 addition & 1 deletion packages/p2p/src/components/advertiser-page/advertiser-page.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
height: fit-content;
width: fit-content;

> span {
> span {
padding-right: 0.8rem;
}

Expand Down
5 changes: 5 additions & 0 deletions packages/p2p/src/components/app.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ const App = props => {
general_store.redirectTo('orders');
order_store.setOrderId(order_id);
}
general_store.props.setP2POrderProps({
order_id,
redirectToOrderDetails: general_store.redirectToOrderDetails,
setIsRatingModalOpen: order_store.setIsRatingModalOpen,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

called setP2POrderProps whenever order_id changes, so when clicking between the different order give feedback buttons, the user can navigate to each order. If this is not here, there will be a bug where the user could navigate from one order to the other, but then wouldn't be able to click the give feedback button as the order_id in p2p_order_props won't be updated

Screenshot 2022-09-21 at 4 58 15 PM

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [order_id]);

Expand Down
5 changes: 5 additions & 0 deletions packages/p2p/src/components/order-details/order-details.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ const OrderDetails = observer(() => {
disposeReactions();
order_store.setOrderPaymentMethodDetails(undefined);
order_store.setOrderId(null);
general_store.props.setP2POrderProps({
order_id: order_store.order_id,
redirectToOrderDetails: general_store.redirectToOrderDetails,
setIsRatingModalOpen: order_store.setIsRatingModalOpen,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

set the p2p_order_props when unmounting order-details page as if the user navigates from the order-details page to another page outside of p2p, then if they click the give feedback button in notifications that has the same order_id as the previous order-details page they had just left, they would not be able to navigate to the same order page. Hence the need to reset the p2p_order_props.

};
}, []); // eslint-disable-line react-hooks/exhaustive-deps

Expand Down
17 changes: 14 additions & 3 deletions packages/p2p/src/stores/general-store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,17 @@ export default class GeneralStore extends BaseStore {
this.updateP2pNotifications(notifications);
}

@action.bound
redirectToOrderDetails(order_id) {
const { order_store } = this.root_store;
this.redirectTo('orders');
this.setOrderTableType(order_list.INACTIVE);
order_store.setOrderId(order_id);
}

@action.bound
showCompletedOrderNotification(advertiser_name, order_id) {
const { order_store } = this.root_store;
const notification_key = `order-${order_id}`;

// we need to refresh notifications in notifications-store in the case of a bug when user closes the notification, the notification count is not synced up with the closed notification
Expand All @@ -240,9 +250,10 @@ export default class GeneralStore extends BaseStore {
this.props.addNotificationMessage({
action: {
onClick: () => {
this.redirectTo('orders');
this.setOrderTableType(order_list.INACTIVE);
this.root_store.order_store.setOrderId(order_id);
if (order_store.order_id === order_id) {
order_store.setIsRatingModalOpen(true);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

have to check if order_id has been set already or else if the user is not on order_details page and click the Give Feedback button, it'll redirect to order_details and the rating modal will open automatically

this.redirectToOrderDetails(order_id);
},
text: localize('Give feedback'),
},
Expand Down