search

AllYourBot / hostedgpt

An open version of ChatGPT you can host anywhere or run locally.
MIT License
344 stars 165 forks source link

Add "Forgot Password" reset feature using Postmark #470

Closed jlvallelonga closed 3 weeks ago

jlvallelonga commented 1 month ago

adds forgot password flow to the app and utilizes Postmark

jlvallelonga commented 1 month ago

still need to move some things around a bit, but this is the general idea

jlvallelonga commented 1 month ago

ok updated everything I think, working on another pr against this one to modify the settings a bit, but that can come in later if we like it:https://github.com/AllYourBot/hostedgpt/pull/476

jlvallelonga commented 1 month ago

system tests are failing with this error:

Error:
Authentications::PasswordTest#test_should_login_as_an_existing_user_and_logout:
Net::ReadTimeout: Net::ReadTimeout with "Net::ReadTimeout with #<TCPSocket:(closed)>"
    test/system/authentications/password_test.rb:6:in `block in <class:PasswordTest>'

I made another PR with just a change to the readme and it failed in the same way (https://github.com/AllYourBot/hostedgpt/pull/481 - was troubleshooting so now it has more than just changes to the readme)

Just updating about where I'm at. Will keep digging

krschacht commented 1 month ago

Dang. I got this CI failure the other day on my PR and I thought it was an issue with github infrastructure. I ran tests 3 times and hit the error each time. Then in the morning I reran the failed tests and it passed.

But I just tried re-running your test suite now and it failed again. Out of curiosity, I went to main (where tests had passed) and re-ran the test suite there and it also failed. I’ve found multiple mentions of this issue online and it’s not encouraging. This sounds complex without a great root cause fix.

Rob — Do you have any ideas about this error?

This thread was a good discussion of the issue but no resolution, and it’s many years old: https://github.com/SeleniumHQ/docker-selenium/issues/198

This issue seemed to explain the root cause is that something during initialization is taking more than 60 seconds: https://github.com/teamcapybara/capybara/issues/1305#issuecomment-112535264 https://github.com/teamcapybara/capybara/issues/1305

This person suggests that we might want to precompile assets: https://stackoverflow.com/questions/57546441/net-readtimeout-occurs-in-rails-systemtest

I found a few mentions online with no useful discussion.

Message ID: @.***>

krschacht commented 1 month ago

Over in this PR https://github.com/AllYourBot/hostedgpt/pull/483 I tried pre-compiling assets and that didn't fix it. I also tried adding a rescue and retry around login_as, since I noticed that's where it was often failing, but the very next time I ran it it failed on a visit ... line.

krschacht commented 4 weeks ago

Hi @jlvallelonga where do we stand on this PR? I see tests are passing so we should be really close. I just checked the diff quickly and I see a couple open comments so I didn’t look through the whole thing (e.g. reverting the CI changes and removing the README addition b/c of that other PR). Let me know if you have other to-do’s on this or if it should be done.

jlvallelonga commented 4 weeks ago

Hi @jlvallelonga where do we stand on this PR? I see tests are passing so we should be really close. I just checked the diff quickly and I see a couple open comments so I didn’t look through the whole thing (e.g. reverting the CI changes and removing the README addition b/c of that other PR). Let me know if you have other to-do’s on this or if it should be done.

Ah sorry. I missed removing that section from the readme. I removed it and the env vars. I'll take another look when I get home tonight, but I think I've got everything now. Ready for merge when you're comfortable with it

krschacht commented 3 weeks ago

I gave it another once over and it all looks great. Merging in now.