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

feat: extract homepage and repository and description from fpm.toml #36

Merged
merged 75 commits into from
Jun 25, 2023

Conversation

henilp105
Copy link
Member

@henilp105 henilp105 commented May 16, 2023

Link for Testing
https://registry-phi.vercel.app/

Thanks and Regards,
Henil

CC: @awvwgk @arteevraina @minhqdao @perazz

@arteevraina
Copy link
Member

@henilp105 Can you provide with testable link ?

@henilp105
Copy link
Member Author

Sure @arteevraina , here is a testable link : https://registry-phi.vercel.app/

@henilp105
Copy link
Member Author

I have patched it. Can you please review it @minhqdao ?

@minhqdao
Copy link
Contributor

minhqdao commented Jun 15, 2023

Loading indicator?

@minhqdao
Copy link
Contributor

Nope, it doesn't work. I'm not even receiving a verification email anymore. Have you checked it yourself if it works?

@henilp105
Copy link
Member Author

sure @minhqdao , let me recheck it once again. I will ping you again for the review , thanks.

@henilp105
Copy link
Member Author

@minhqdao , I have tested It . can you please review it ?

@minhqdao
Copy link
Contributor

Can't log into any of my accounts anymore. I created a new one, but the password wasn't registered by Safari's password manager, so I verified the mail and wanted to reset the password. Now it says "Please verify your email". But it is verified.

I don't feel like this is all working well. Please take your time to check all the authentication flows and make sure it all works well to the best of your knowledge before requesting reviews.

@henilp105
Copy link
Member Author

Sure @minhqdao , I have patched it and you would have to create a new account as I had to clear the entire db to add a new paramter to the users collection for the new unverified email, can you please review it again ?

Testable link: https://registry-phi.vercel.app/

@minhqdao
Copy link
Contributor

Cannot change password. Still the old one is valid.

@henilp105
Copy link
Member Author

Thanks @minhqdao , good catch , I have patched this up.

@minhqdao
Copy link
Contributor

Have you tested it yourself?

@henilp105
Copy link
Member Author

yes @minhqdao , I have tested.

@minhqdao
Copy link
Contributor

"Change Password" still doesn't work for me.

@henilp105
Copy link
Member Author

@minhqdao I have fixed that , the password was being changed , just the notification on the user side had been disabled, I have enabled it. Can you please review it again ?

@minhqdao
Copy link
Contributor

minhqdao commented Jun 23, 2023

Oh, I see now that the UI doesn't show any error response, which had caused the problem.

If I attempt to change the password and the old password I entered wasn't correct, I do not get a response.

First off, and as mentioned many times in this PR:

  1. Have a loading indicator that indicates that the request is currently being processed.

Then:

  1. The problem here was that when I entered an incorrect old password, I do not get an error message. I, in fact, get no message at all, which might imply that everything worked normally.

@henilp105
Copy link
Member Author

@minhqdao Thanks for the review , i have fixed them. Can you please review it again ?

Copy link
Contributor

@minhqdao minhqdao left a comment

Choose a reason for hiding this comment

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

It's working now, thank you. I'll approve this now, yet there is a small thing you could change in this PR or in a follow-up. The login screen says "Please enter your username/email and password to log in." but the hint text in the text field still says "Email". Would be great if we also change it to "Email or username".

@henilp105 henilp105 merged commit 4705f7b into fortran-lang:main Jun 25, 2023
@minhqdao
Copy link
Contributor

I see the code change but the input field still says "Email" only. I did clear the cache. Is it because it hasn't been deployed yet?

@henilp105
Copy link
Member Author

yes exactly, I have deployed it now. @minhqdao

@minhqdao
Copy link
Contributor

Maybe you can set up an automatic deployment so this doesn't happen in the future.

@henilp105 henilp105 mentioned this pull request Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants