@tordans commented on this pull request.


> @@ -14,6 +14,7 @@ class ConfirmationsController < ApplicationController
   before_action :require_cookies, :only => [:confirm]
 
   def confirm
+    # The post happens in confirm.html.erb and user.js

Please feel free to improve the wording and even remove this line.

I want to point out, however, that in my understanding the form in 
`confirm.html.erb` and the JS in `user.js` that auto-submits the form is 
tightly coupled to the `if request.post?` in `#confirm` 
https://github.com/openstreetmap/openstreetmap-website/pull/6138/files/0e160369a76b968b4fc71468f06506aabe8693c7#diff-961c5d7e0310073ca00d502eda2687808102441be660a654cab29645fbcea019R18
 
So relevant changes to the flow will have to touch this code and give an 
opportunity to update the comment.

I can say that when I spend about 1.5 hours pairing on this issue (2 people: 3 
h) we did not expect this kind of flow where an OAuth `POST` gets redirected to 
a html page which auto submits a form to step into the `if request.post?` 
branch. It took us quite a while to figure this out and a hint like this would 
have speed things up for us a lot.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6138#discussion_r2173116294
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/6138/review/2968355...@github.com>
_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to