[openstreetmap/openstreetmap-website] Welcome screen shows "Continue with authorization" when OSM account created from authorization flow (PR #4329)

[Tom Hughes]

@tomhughes commented on this pull request.



> @@ -317,6 +305,24 @@ def auth_failure
 
   private
 
+  def welcome_options
+    uri = URI(session[:referer]) if session[:referer].present?
+    welcome_options = {}
+    welcome_options["oauth_return_url"] = session[:referer] if uri&.path == oauth_authorization_path

Is there a reason for going back to `session[:referer]` for the assignment here instead of just using `uri`?

> @@ -317,6 +305,24 @@ def auth_failure
 
   private
 
+  def welcome_options
+    uri = URI(session[:referer]) if session[:referer].present?
+    welcome_options = {}
+    welcome_options["oauth_return_url"] = session[:referer] if uri&.path == oauth_authorization_path
+
+    begin
+      %r{map=(.*)/(.*)/(.*)}.match(uri.fragment) do |m|
+        editor = Rack::Utils.parse_query(uri.query).slice("editor")
+        welcome_options = { "zoom" => m[1],
+                            "lat" => m[2],
+                            "lon" => m[3] }.merge(editor).merge(welcome_options)

Do we need to merge with the existing options here? Does it ever make sense to have both an OAuth return URL and coordinates?

Should this whole thing be two separate paths for setting the options based on whether the referer is the oauth return URL or not?

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

Message ID: <openstreetmap/openstreetmap-website/pull/4329/review/1730077758 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20231114/76877b67/attachment.htm>