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.

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: sign-in/sign-up form shows success even if API was not sent successfully [SPMVP-6214] #187

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

arielllin
Copy link
Contributor

@arielllin arielllin commented Nov 1, 2023

Jira

https://storipress-media.atlassian.net/browse/SPMVP-6214

Root cause

subscribe 的 submit button 按下後會呼叫 paywall 的 handleSignup,handleSignup 這邊只有判斷如果是已經註冊過的使用者在呼叫 login 後就直接顯示 dialog,並沒有判斷是否有呼叫成功

Purpose

修正即使未登入成功仍會顯示成功訊息

For who

  • reader-facing?
  • publisher-facing?
  • developer-facing?

How did you fix it?

如果呼叫 sign-in/sign-up api 未成功,顯示錯誤提示視窗

Production deployment notes

🎩 Tophat

Do a thorough 🎩. What is tophatting?

Consider testing:

  • Existing functionality which could break due to your changes (e.g. global changes)
  • New functionality being introduced. Ensure it meets intended behavior
  • All different permutations (flows, states, conditions, etc) that are possible
  • If you are modifying something which is used in multiple places, 🎩 all affected areas not just what you intend to change

🎩 Instructions

2023-11-01.12.26.24.mov

Related PRs

Checklist before requesting review

  • I have done a self-review of my own code (comment on code is not required but recommended)
  • I did the Tophat steps and confirm the issue fixed
  • I have confirmed there is no issue reported by Static Analyzer (Please double check for custom class. For other issues, if you think it's ignorable, please add // eslint-ignore-next-line <rule name> on it)
  • I have confirmed there is no deepsource issue (if you think it's ignorable, please explain why and add a skipcq comment)
  • I confirmed that the unit test is passed (if you break it, please fix it in this PR)
  • I confirmed that I didn't break E2E test (if you break it, please fix it or create a new Jira)

Emoji Guide

For reviewers: Emojis can be added to comments to call out blocking versus non-blocking feedback.

E.g: Praise, minor suggestions, or clarifying questions that don’t block merging the PR.

🟢 Nice refactor!

🟡 Why was the default value removed?

E.g: Blocking feedback must be addressed before merging.

🔴 This change will break something important

Blocking 🔴 ❌ 🚨 RED
Non-blocking 🟡 💡 🤔 💭 Yellow, thinking, etc
Praise 🟢 💚 😍 👍 🙌 Green, hearts, positive emojis, etc

Gif (optional)

image

@arielllin arielllin requested a review from DanSnow November 1, 2023 04:44
@DanSnow DanSnow merged commit a433d90 into master Nov 1, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants