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

Remove update and forgot password APIs #6591

Merged
merged 1 commit into from
Sep 14, 2023
Merged

Remove update and forgot password APIs #6591

merged 1 commit into from
Sep 14, 2023

Conversation

idanovo
Copy link
Contributor

@idanovo idanovo commented Sep 13, 2023 •
edited
Loading

Closes #6532

@idanovo idanovo self-assigned this Sep 13, 2023
@idanovo idanovo linked an issue Sep 13, 2023 that may be closed by this pull request
API cleanup: update/forgot password removal #6532
Closed
@idanovo idanovo added area/API Improvements or additions to the API v1.0.0-blocker Issues that should be closed before going out with v1.0.0 labels Sep 13, 2023
@github-actions
Copy link

github-actions bot commented Sep 13, 2023 •
edited
Loading

♻️ PR Preview 7ff3685 has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

@idanovo idanovo added the include-changelog PR description should be included in next release changelog label Sep 13, 2023
arielshaqed
arielshaqed approved these changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

Neat, thanks!

  • login.tsx needs some care, GitHub Actions / Analyze says maybe the getAuthCapabilities() call is unused.

api/swagger.yml
@@ -691,15 +691,13 @@ components:
properties:
id:
type: string
description: a unique identifier for the user. In password-based authentication, this is the email.
description: a unique identifier for the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch! ✨

arielshaqed
arielshaqed requested changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

Forgot, so blocking for now

ALSO please go over webui/ and look for these URLs: webui doesn't use a generated client but did use these URLs, so needs to be changed manually and the TypeScript compiler cannot find the change from the OpenAPI spec change.

@idanovo
Copy link
Contributor Author

idanovo commented Sep 13, 2023

Neat, thanks!

  • login.tsx needs some care, GitHub Actions / Analyze says maybe the getAuthCapabilities() call is unused.

@arielshaqed I plan to change getAuthCapabilities to be an inner API in a different PR

@idanovo
Copy link
Contributor Author

idanovo commented Sep 13, 2023

Forgot, so blocking for now

ALSO please go over webui/ and look for these URLs: webui doesn't use a generated client but did use these URLs, so needs to be changed manually and the TypeScript compiler cannot find the change from the OpenAPI spec change.

@arielshaqed Not sure I understand this comment. I removed the calls for update/forgot password from the webui code.
Am I missing anything?

@idanovo idanovo requested a review from arielshaqed September 13, 2023 15:14
@arielshaqed
Copy link
Contributor

Forgot, so blocking for now
ALSO please go over webui/ and look for these URLs: webui doesn't use a generated client but did use these URLs, so needs to be changed manually and the TypeScript compiler cannot find the change from the OpenAPI spec change.

@arielshaqed Not sure I understand this comment. I removed the calls for update/forgot password from the webui code. Am I missing anything?

Nope, you're right and I'm wrong.

THANKS!

arielshaqed
arielshaqed approved these changes Sep 14, 2023
View reviewed changes
Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

Love the diff stats!
GitHub shows 22 lines "added", 2454 removed

@idanovo idanovo merged commit 9124b1f into master Sep 14, 2023
36 of 42 checks passed
@idanovo idanovo deleted the 6532-remove-update-and-forgot-password-api branch September 14, 2023 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arielshaqed arielshaqed arielshaqed approved these changes

@nopcoder nopcoder Awaiting requested review from nopcoder

Assignees

@idanovo idanovo

Labels
area/API Improvements or additions to the API include-changelog PR description should be included in next release changelog v1.0.0-blocker Issues that should be closed before going out with v1.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

API cleanup: update/forgot password removal
2 participants
@idanovo @arielshaqed