[FEQ] Ameerul / FEQ-872 / Getting console error while adding payment method in P2P #11293

ameerul-deriv · 2023-11-09T03:28:32Z

Changes:

Please provide a summary of the change.

passed missing value prop which is causing the console log issues
fixed the console log issue when the user finishes editing their advert by removing observer from the component.
added useP2PAdvertiserPaymentMethods hook's update function instead of using my-profile-store's updatePaymentMethod function
Deleted updatePaymentMethod from my-profile-store
Also fixed console error showing after clicking on adding payment method in buy sell modal.
Issue was because we are passing a component as a title which is wrapped by Text, so instead opted to pass in the string only
Also added other props for return icon and onReturn functions.

Screenshots:

Please provide some screenshots of the change.

Staging console errors while adding/editing payment method

Screen.Recording.2023-11-09.at.11.24.30.AM.mov

No more console errors while adding/editing payment method

Screen.Recording.2023-11-09.at.11.25.48.AM.mov

Previous Console Error related to payment method in buy sell modal

Screen.Recording.2023-11-30.at.11.44.19.AM.mov

No more error

Screen.Recording.2023-11-30.at.11.44.52.AM.mov

vercel · 2023-11-09T03:28:36Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
deriv-app	✅ Ready (Inspect)	Visit Preview	Nov 30, 2023 4:00am

ameerul-deriv · 2023-11-09T03:30:46Z

...s/p2p/src/pages/my-profile/payment-methods/payment-methods-list/edit-payment-method-form.jsx

 };

-export default observer(EditPaymentMethodForm);
+export default EditPaymentMethodForm;


Wrapping this component with observer caused the console logging error issue, which is caused by updating the payment method, which is then updating any observables inside this component whilst it is unmounting.

We don't need to have observer wrapping this component since it doesn't need to listen to any changes except if there are errors whilst updating the payment method

coveralls · 2023-11-09T03:42:27Z

coverage: 29.024%. remained the same
when pulling 038ced9 on ameerul-deriv:FEQ-872-getting-console-error-when-adding-payment-method
into 36d1841 on binary-com:master.

github-actions · 2023-11-09T03:44:26Z

A production App ID was automatically generated for this PR. (log)

PR: [FEQ] Ameerul / FEQ-872 / Getting console error while adding payment method in P2P #11293
URLs:
- w/ App ID + Server: https://deriv-app-git-fork-ameerul-deriv-feq-872-getting-console-80afee.binary.sx?qa_server=red.derivws.com&app_id=32761
- Original: https://deriv-app-git-fork-ameerul-deriv-feq-872-getting-console-80afee.binary.sx
App ID: 32761

Click here to copy & paste above information.

- **PR**: [https://github.com/binary-com/deriv-app/pull/11293](https://github.com/binary-com/deriv-app/pull/11293)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-ameerul-deriv-feq-872-getting-console-80afee.binary.sx?qa_server=red.derivws.com&app_id=32761
    - **Original**: https://deriv-app-git-fork-ameerul-deriv-feq-872-getting-console-80afee.binary.sx
- **App ID**: `32761`

github-actions · 2023-11-09T03:44:55Z

🚨 Lighthouse report for the changes in this PR:

Category	Score
🔺 Performance	16
🟧 Accessibility	75
🟢 Best practices	92
🟢 SEO	92
🟧 PWA	80

Lighthouse ran with https://deriv-app-git-fork-ameerul-deriv-feq-872-getting-console-80afee.binary.sx/

...ages/p2p/src/pages/my-profile/payment-methods/add-payment-method/add-payment-method-form.jsx

chrisgodfrey-deriv · 2023-11-09T19:27:30Z

Please use the [FEQ] prefix for the PR title
Nice PR description though!

…-getting-console-error-when-adding-payment-method

…odal

gitguardian · 2023-11-30T03:43:11Z

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	Secret	Commit	Filename
-	Generic High Entropy Secret	`3134575`	nx.json	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secret safely. Learn here the best practices.
Revoke and rotate this secret.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}

sonarqubecloud · 2023-11-30T03:46:28Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
7.0% Duplication

ameerul-deriv · 2023-11-30T03:59:43Z

packages/p2p/src/components/modal-manager/modals/buy-sell-modal/buy-sell-modal.tsx

+                    onReturn={onReturn}
                    portalId='modal_root'
-                    title={<BuySellModalTitle is_buy={is_buy_advert} />}
+                    title={getModalTitle()}


I opted to pass a string instead of the component because this is what caused the console error issues. Because we had passed it as a component, the title prop in Modal component is wrapped inside Text which is strange as we had previously returned it as a Text Component.

We could create another prop for specifically passing a component, but the styles are already there, and we don't need to repeat it again and style it ourselves from the child to the parent.

There's no issue with the MobileFullPageModal, so I had left it as it is

github-actions · 2023-11-30T04:04:56Z

⏳ Generating Lighthouse report...

github-actions · 2023-11-30T06:57:09Z

❌ Smoke test run (1) failed. See logs for details: Visit Action

github-actions · 2023-11-30T06:57:37Z

❌ Smoke test run (2) failed. See logs for details: Visit Action

github-actions · 2023-11-30T06:57:38Z

❌ Smoke test run (1) failed. See logs for details: Visit Action

fix: console errors for add and edit payment method

d755a63

ameerul-deriv requested review from farrah-deriv and nijil-deriv as code owners November 9, 2023 03:28

boring-cyborg bot added the P2P label Nov 9, 2023

ameerul-deriv deployed to Preview November 9, 2023 03:28 — with GitHub Actions Active

ameerul-deriv deployed to Preview November 9, 2023 03:28 — with GitHub Actions View deployment

ameerul-deriv commented Nov 9, 2023

View reviewed changes

farhan-nurzi-deriv previously approved these changes Nov 9, 2023

View reviewed changes

farrah-deriv reviewed Nov 9, 2023

View reviewed changes

...ages/p2p/src/pages/my-profile/payment-methods/add-payment-method/add-payment-method-form.jsx Show resolved Hide resolved

farrah-deriv previously approved these changes Nov 9, 2023

View reviewed changes

suisin-deriv approved these changes Nov 9, 2023

View reviewed changes

ameerul-deriv changed the title ~~Ameerul /FEQ-872 Getting console error while adding payment method in P2P~~ [FEQ] Ameerul /FEQ-872 Getting console error while adding payment method in P2P Nov 10, 2023

ameerul-deriv added 3 commits November 30, 2023 10:17

Merge branch 'master' of github.com:binary-com/deriv-app into FEQ-872…

3134575

…-getting-console-error-when-adding-payment-method

Merge branch 'master' of github.com:binary-com/deriv-app into FEQ-872…

f73fa28

…-getting-console-error-when-adding-payment-method

fix: console log when clicking on adding payment method in buy-sell-m…

038ced9

…odal

ameerul-deriv dismissed stale reviews from farrah-deriv and farhan-nurzi-deriv via 038ced9 November 30, 2023 03:42

ameerul-deriv requested review from ali-hosseini-deriv, maryia-deriv and matin-deriv as code owners November 30, 2023 03:42

boring-cyborg bot added the Components label Nov 30, 2023

ameerul-deriv temporarily deployed to Preview November 30, 2023 03:42 — with GitHub Actions Inactive

ameerul-deriv temporarily deployed to Preview November 30, 2023 03:43 — with GitHub Actions Inactive

ameerul-deriv commented Nov 30, 2023

View reviewed changes

ameerul-deriv changed the title ~~[FEQ] Ameerul /FEQ-872 Getting console error while adding payment method in P2P~~ [FEQ] Ameerul /FEQ-872/Getting console error while adding payment method in P2P Nov 30, 2023

ameerul-deriv changed the title ~~[FEQ] Ameerul /FEQ-872/Getting console error while adding payment method in P2P~~ [FEQ] Ameerul / FEQ-872 / Getting console error while adding payment method in P2P Nov 30, 2023

farrah-deriv approved these changes Nov 30, 2023

View reviewed changes

farrah-deriv merged commit 73cdb3a into deriv-com:master Dec 4, 2023

[FEQ] Ameerul / FEQ-872 / Getting console error while adding payment method in P2P #11293

[FEQ] Ameerul / FEQ-872 / Getting console error while adding payment method in P2P #11293

Uh oh!

Conversation

ameerul-deriv commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Screenshots:

Staging console errors while adding/editing payment method

No more console errors while adding/editing payment method

Previous Console Error related to payment method in buy sell modal

No more error

Uh oh!

vercel bot commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ameerul-deriv Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

coveralls commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

chrisgodfrey-deriv commented Nov 9, 2023

Uh oh!

gitguardian bot commented Nov 30, 2023

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Uh oh!

sonarqubecloud bot commented Nov 30, 2023

Uh oh!

ameerul-deriv Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 30, 2023

Uh oh!

github-actions bot commented Nov 30, 2023

Uh oh!

github-actions bot commented Nov 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ameerul-deriv commented Nov 9, 2023 •

edited

Loading

vercel bot commented Nov 9, 2023 •

edited

Loading

coveralls commented Nov 9, 2023 •

edited

Loading

github-actions bot commented Nov 9, 2023 •

edited

Loading

github-actions bot commented Nov 9, 2023 •

edited

Loading

github-actions bot commented Nov 30, 2023 •

edited

Loading