Re: [openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)

2024-10-15 Thread Nenad Vujicic via rails-dev 
@nenad-vujicic approved this pull request.

This works great on my side, thank you very much!

Wild idea: since we already have decision procedure, which answers the question 
"do we have multiple pages?", why don't we use it for hiding pagination buttons 
when they are not required / usable?



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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)

2024-10-15 Thread Anton Khorev via rails-dev 
@AntonKhorev commented on this pull request.



>  <% else %>
+  <%= form_tag(url_for("controller" => "notes", "action" => "index"), "method" 
=> :get, "data" => { "turbo" => true, "turbo-frame" => "pagination", 
"turbo-action" => "advance" }) do %>
+
+  
+<%= label_tag :status, t(".status") %>
+<%= select_tag  :status,
+options_for_select([[t(".all"), "all"], [t(".open"), 
"open"], [t(".closed"), "closed"]], params[:status]),
+:include_blank => t(".select_status"),

Why do you need `:include_blank`s? You have labels that say what these selects 
are and the filter acts as if the first options are selected, but you don't 
show them as selected.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)

2024-10-15 Thread Anton Khorev via rails-dev 
About the date pickers:

I wanted to add them to our standard pagination because we already have date 
indexes. Then we wouldn't need them here, specifically in this notes filer. But 
this plan haven't worked so far because:

- I wanted to first add a simpler thing to the standard pagination, namely 
links to first/last pages. I made several versions of this feature - #4710 
#4729 #4733 #4734 - but none of them got much attention from the maintainers. I 
can still choose to add date pickers, but this will give me one more pull 
request that's going to be in conflict with the others which I'll have to 
maintain.
- As you can see this notes list doesn't even use the standard pagination. I 
tried doing that too #4532. That wasn't convincing enough because it was too 
complicated. The complications were there because I used note ids as a cursor 
but I couldn't sort by them, this wouldn't get notes in the last updated order. 
The suggested solution by one of the maintainers is to use last comment ids 
instead, but it's currently being undermined by another maintainer who wants to 
remove opening comments.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Support versions in elements multi fetch (PR #3715)

2024-10-15 Thread mmd via rails-dev 
@mmd-osm approved this pull request.





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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)

2024-10-15 Thread Anton Khorev via rails-dev 
@AntonKhorev commented on this pull request.



> @@ -91,7 +91,7 @@ en:
 support_url: Support URL
 allow_read_prefs:  read their user preferences
 allow_write_prefs: modify their user preferences
-allow_write_diary: create diary entries, comments and make friends
+allow_write_diary: create diary entries, comments and follow users

This permission is not used now but if it was, would it make sense to bundle 
follow/unfollow together with diary editing/commenting? Maybe just remove 
following from here?

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)

2024-10-15 Thread Anton Khorev via rails-dev 
@AntonKhorev commented on this pull request.



>  <% else %>
+  <%= form_tag(url_for("controller" => "notes", "action" => "index"), "method" 
=> :get, "data" => { "turbo" => true, "turbo-frame" => "pagination", 
"turbo-action" => "advance" }) do %>
+
+  
+<%= label_tag :status, t(".status") %>
+<%= select_tag  :status,
+options_for_select([[t(".all"), "all"], [t(".open"), 
"open"], [t(".closed"), "closed"]], params[:status]),
+:include_blank => t(".select_status"),
+:class => "form-select" %>
+  
+  
+<%= label_tag :note_type, t(".note_type") %>

I wouldn't call this a "note type". It's an interaction type between the user 
and the note.

And then there's a question if this kind of filtering can be done efficiently 
because here you have `INNER JOIN "note_comments"` twice.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

[openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)

2024-10-15 Thread David Tsiklauri via rails-dev 
This PR addresses "Modify the way 'Friends' are added" issue 
mentioned in https://github.com/openstreetmap/openstreetmap-website/issues/3310

This PR replaces all occurrences of "friends" to 
"followers", including texts, tests and URLs.

This PR was not broken down to smaller pieces to maintain consistency and avoid 
different names of the same functionality.
You can view, comment on, or merge this pull request online at:

  https://github.com/openstreetmap/openstreetmap-website/pull/5261

-- Commit Summary --

  * Modify the way Friends are added

-- File Changes --

M app/abilities/ability.rb (2)
M app/controllers/diary_entries_controller.rb (2)
M app/controllers/friendships_controller.rb (12)
M app/helpers/changesets_helper.rb (2)
M app/mailers/user_mailer.rb (2)
M app/views/dashboards/_contact.html.erb (6)
M app/views/dashboards/show.html.erb (10)
R app/views/friendships/follow_user.html.erb (0)
R app/views/friendships/unfollow_user.html.erb (0)
M app/views/user_mailer/friendship_notification.html.erb (4)
M app/views/user_mailer/friendship_notification.text.erb (4)
M app/views/users/show.html.erb (4)
M config/locales/en.yml (54)
M config/routes.rb (4)
M test/controllers/friendships_controller_test.rb (84)
M test/controllers/oauth2_authorized_applications_controller_test.rb (2)
M test/system/dashboard_test.rb (8)
M test/system/friendships_test.rb (8)

-- Patch Links --

https://github.com/openstreetmap/openstreetmap-website/pull/5261.patch
https://github.com/openstreetmap/openstreetmap-website/pull/5261.diff

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Remove Nominatim prefixes mention in contributors guide (PR #5260)

2024-10-15 Thread Anton Khorev via rails-dev 
Merged #5260 into master.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Remove Nominatim prefixes mention in contributors guide (PR #5260)

2024-10-15 Thread Anton Khorev via rails-dev 
Merged, thanks.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Share button should preserve the node/way/relation id when creating the URL (Issue #5252)

2024-10-15 Thread Anton Khorev via rails-dev 
How do you want the html embed to behave in this case?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5252#issuecomment-2413995449
You are receiving this because you are subscribed to this thread.

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

[openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)

2024-10-15 Thread Anton Khorev via rails-dev 
![image](https://github.com/user-attachments/assets/1753052d-07d2-4f4a-8b11-092b327b73f3)

You can view, comment on, or merge this pull request online at:

  https://github.com/openstreetmap/openstreetmap-website/pull/5262

-- Commit Summary --

  * Add older/newer links in front of diary, comment, issue, block pages

-- File Changes --

M app/views/diary_comments/_page.html.erb (4)
M app/views/diary_entries/_page.html.erb (4)
M app/views/issues/_page.html.erb (5)
M app/views/user_blocks/_page.html.erb (4)

-- Patch Links --

https://github.com/openstreetmap/openstreetmap-website/pull/5262.patch
https://github.com/openstreetmap/openstreetmap-website/pull/5262.diff

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)

2024-10-15 Thread Anton Khorev via rails-dev 
@AntonKhorev pushed 1 commit.

74be3f8b7c4fa192b8f08dcc6d11d88afbcf3c1a  Add older/newer links in front of 
diary, comment, issue, block pages

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5262/files/53fb10f9cc28a7017e423baa5a09b5376ea2ed47..74be3f8b7c4fa192b8f08dcc6d11d88afbcf3c1a
You are receiving this because you are subscribed to this thread.

Message ID: 

___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)

2024-10-15 Thread Anton Khorev via rails-dev 
@AntonKhorev pushed 1 commit.

0b053314fd7f427f2b5c8a0c35b583bda43ce0f3  Add older/newer links in front of 
diary, comment, issue, block pages

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5262/files/74be3f8b7c4fa192b8f08dcc6d11d88afbcf3c1a..0b053314fd7f427f2b5c8a0c35b583bda43ce0f3
You are receiving this because you are subscribed to this thread.

Message ID: 

___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)

2024-10-15 Thread Tom Hughes via rails-dev 
@tomhughes commented on this pull request.



> @@ -280,8 +280,8 @@
   resource :profile, :only => [:edit, :update]
 
   # friendships
-  match "/user/:display_name/make_friend" => "friendships#make_friend", :via 
=> [:get, :post], :as => "make_friend"
-  match "/user/:display_name/remove_friend" => "friendships#remove_friend", 
:via => [:get, :post], :as => "remove_friend"
+  match "/user/:display_name/follow_user" => "friendships#follow_user", :via 
=> [:get, :post], :as => "follow_user"
+  match "/user/:display_name/unfollow_user" => "friendships#unfollow_user", 
:via => [:get, :post], :as => "unfollow_user"

I think the URL here could just be `/follow` and `/unfollow` and we don't 
really need the `_user` suffix? Possibly the actions should be `create` and 
`destroy` though? Probably the controller should be renamed to though I'm not 
sure what to...

>
 
 
-  <%= render :partial => "contact", :collection => friends, :locals => 
{ :type => "friend" } %>
+  <%= render :partial => "contact", :collection => friends, :locals => 
{ :type => "followed user" } %>

This is purely internal, and not user visible, but a simple `followed` probably 
suffices as a type?

> @@ -1603,12 +1603,12 @@ en:
   footer_html: "You can also read the message at %{readurl} and you can 
send a message to the author at %{replyurl}"
 friendship_notification:
   hi: "Hi %{to_user},"
-  subject: "[OpenStreetMap] %{user} added you as a friend"
-  had_added_you: "%{user} has added you as a friend on OpenStreetMap."
+  subject: "[OpenStreetMap] %{user} followed you"
+  had_followed_you: "%{user} has followed you on OpenStreetMap."

I'm not sure why this key has a `had_` prefix but it was always incorrect so 
I'd suggest just dropping it and using `followed_you`.

> @@ -91,7 +91,7 @@ en:
 support_url: Support URL
 allow_read_prefs:  read their user preferences
 allow_write_prefs: modify their user preferences
-allow_write_diary: create diary entries, comments and make friends
+allow_write_diary: create diary entries, comments and follow users

Agreed as that is never used and there's no good reason to include that Id just 
drop it from the comment and if we ever want a permission for managing follows 
we can add that separately.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)

2024-10-15 Thread mmd via rails-dev 
@mmd-osm commented on this pull request.



> @@ -1603,12 +1603,12 @@ en:
   footer_html: "You can also read the message at %{readurl} and you can 
send a message to the author at %{replyurl}"
 friendship_notification:
   hi: "Hi %{to_user},"
-  subject: "[OpenStreetMap] %{user} added you as a friend"
-  had_added_you: "%{user} has added you as a friend on OpenStreetMap."
+  subject: "[OpenStreetMap] %{user} followed you"
+  had_followed_you: "%{user} has followed you on OpenStreetMap."

I would probably expect a slightly different wording here, like "%{user} is 
following you on OpenStreetMap now.". 

"has followed you" sounds to me like this is a thing of the past, and not 
necessarily the case anymore.



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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Share button should preserve the node/way/relation id when creating the URL (Issue #5252)

2024-10-15 Thread 積丹尼 Dan Jacobson via rails-dev 
> How do you want the html embed to behave in this case?

Naturally it should look just like the picture I posted.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5252#issuecomment-2415783536
You are receiving this because you are subscribed to this thread.

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

[openstreetmap/openstreetmap-website] Bump coverallsapp/github-action from 2.3.0 to 2.3.2 (PR #5263)

2024-10-15 Thread dependabot[bot] via rails-dev 
Bumps 
[coverallsapp/github-action](https://github.com/coverallsapp/github-action) 
from 2.3.0 to 2.3.2.


Release notes

Sourced from coverallsapp/github-action's
 releases.




v2.3.2


What's Changed



New Contributors



Full Changelog: https://github.com/coverallsapp/github-action/compare/v2.3.1...v2.3.2
;

v2.3.1


What's Changed


Extend behavior of fail-on-error option to 
setup failures by @​afinetooth;
 in coverallsapp/github-action#226
;

    

  • 

    Technically an enhancement, these changes make the action behave as 
many customers already expect by ignoring any and all 
failures when the fail-on-error input is set to 
false.
    

    • 

  • 

    Adds logic to handle any failures in "setup" tasks, 
including downloading the coverage-reporter binary, 
verifying the binary, and finding the binary by its expected name after 
extraction.
    

    • 

  • 

    The new logic checks these actions and exits with code 
1 on failure, except if 
fail-on-error is set to true, 
in which case it returns exit code 0.
    

    • 

  • 

    Adds a matrix workflow that tests the action for each 
os and the two key binary commands 
(coveralls report and coevralls 
done). Each of these scenarios implicitly tests our setup tasks 
since they run first in each scenario.
    

    • 

  • 

    Also extends the behavior of debug: true to 
flip the shell-specific debug flag for each os 
including set -x for linux 
and macos and Set-PSDebug -Trace 
1 for windows.
    

    • 



Full Changelog: https://github.com/coverallsapp/github-action/compare/v2.3.0...v2.3.1
;






Commits

    

  • 43f11c4;
 Verify that coverage-reporter-version option is 
recognized (#229;)
    • 

  • c258231;
 Add build

Re: [openstreetmap/openstreetmap-website] Bump coverallsapp/github-action from 2.3.0 to 2.3.1 (PR #5256)

2024-10-15 Thread dependabot[bot] via rails-dev 
Closed #5256.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Bump coverallsapp/github-action from 2.3.0 to 2.3.1 (PR #5256)

2024-10-15 Thread dependabot[bot] via rails-dev 
Superseded by #5263.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Re: [openstreetmap/openstreetmap-website] Bump coverallsapp/github-action from 2.3.0 to 2.3.2 (PR #5263)

2024-10-15 Thread Tom Hughes via rails-dev 
Merged #5263 into master.

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

Message ID: 
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev