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

Mock OTP and phone numbers #7565

Merged
merged 24 commits into from
Jul 3, 2024
Merged

Mock OTP and phone numbers #7565

merged 24 commits into from
Jul 3, 2024

Conversation

christyjacob4
Copy link
Member

@christyjacob4 christyjacob4 commented Feb 11, 2024

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@eldadfux eldadfux changed the title feat: initial commit Mock OTP and phone numbers Feb 18, 2024
@eldadfux eldadfux added product / auth Fixes and upgrades for the Appwrite Auth / Users / Teams services. version / latest labels Feb 18, 2024
@christyjacob4 christyjacob4 changed the base branch from main to 1.6.x June 20, 2024 15:20
@christyjacob4 christyjacob4 changed the base branch from 1.6.x to sync-with-main June 20, 2024 19:25
Base automatically changed from sync-with-main to 1.6.x June 25, 2024 15:13
app/console Outdated Show resolved Hide resolved
app/init.php Outdated Show resolved Hide resolved
app/init.php Outdated Show resolved Hide resolved
@loks0n loks0n self-requested a review July 1, 2024 13:45
src/Appwrite/Utopia/Response/Model/Project.php Outdated Show resolved Hide resolved
src/Appwrite/Utopia/Response.php Show resolved Hide resolved
*
* Validates if a given object represents a valid phone and OTP pair
*/
class MockNumber extends Validator
Copy link
Member

Choose a reason for hiding this comment

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

Is MockNumber a good name here?

For me it's not descriptive enough...
but it's a tricky one

Some ideas:

MockPhoneOTPList
MockSMSAuthConfiguration
OTPTestingConfiguration

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this validator is confusing, if we have flat parameters, we can avoid this altogether

Copy link
Contributor

Choose a reason for hiding this comment

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

MockPhoneOTPList makes most sense to me

Copy link
Member

Choose a reason for hiding this comment

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

Let's solve this with solution for improving endpoint to make the parameter granular

app/controllers/api/projects.php Show resolved Hide resolved
*
* Validates if a given object represents a valid phone and OTP pair
*/
class MockNumber extends Validator
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this validator is confusing, if we have flat parameters, we can avoid this altogether

src/Appwrite/Utopia/Response/Model/MockNumber.php Outdated Show resolved Hide resolved
Copy link
Member

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

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

Let's fix the endpoint for adding mock number during QA. Merging now to proceed to QA

@lohanidamodar lohanidamodar merged commit 71a9694 into 1.6.x Jul 3, 2024
5 checks passed
@lohanidamodar lohanidamodar deleted the mock-numbers branch July 3, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product / auth Fixes and upgrades for the Appwrite Auth / Users / Teams services.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants