Re: [openstreetmap/openstreetmap-website] Always show changeset element page links below headings (PR #5213)

2024-09-15 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5213#issuecomment-2351708027
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] Fix browse elements timeouts (PR #5214)

2024-09-15 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5214#issuecomment-2351709374
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] Some more i18n unused translation fixes (PR #5217)

2024-09-15 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5217#issuecomment-2351715900
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] Some more i18n unused translation fixes (PR #5217)

2024-09-15 Thread Tom Hughes via rails-dev
Merged #5217 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5217#event-14264105629
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] Fix browse elements timeouts (PR #5214)

2024-09-15 Thread Tom Hughes via rails-dev
Merged #5214 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5214#event-14264105172
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] Always show changeset element page links below headings (PR #5213)

2024-09-15 Thread Tom Hughes via rails-dev
Merged #5213 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5213#event-14264105544
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] Displays links for friend options on user profile only if user is signed in (PR #5203)

2024-09-15 Thread Tom Hughes via rails-dev
@tomhughes approved this pull request.

Thanks - that looks good now.



-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5203#pullrequestreview-2305543018
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] buttons on userpages visible after logging out (Issue #5199)

2024-09-15 Thread Tom Hughes via rails-dev
Closed #5199 as completed via 8d77891fd6796f32f665f7524e0ed8f098507f6d.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5199#event-14264175175
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] Displays links for friend options on user profile only if user is signed in (PR #5203)

2024-09-15 Thread Tom Hughes via rails-dev
Merged #5203 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5203#event-14264175180
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] Drop creation_ip column (PR #5212)

2024-09-15 Thread Tom Hughes via rails-dev
@tomhughes pushed 3 commits.

fe96c0a524c9ce092eed8e8ac14e6f9309e778cc  Replace creation_ip with 
creation_address
8bb463318d98db6c9b24e3a1d9de403e916cdf28  Ignore the creation_ip column which 
is no longer used
5f7e177e99654e4b21fe5c6f5dc4228a218ae55d  Drop creation_ip column

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5212/files/65c4ade1c01bb0f2bf30b4f3a3f06690626f543b..5f7e177e99654e4b21fe5c6f5dc4228a218ae55d
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] Replace creation_ip with creation_address (PR #5218)

2024-09-15 Thread Tom Hughes via rails-dev
This replaces `creation_ip` with `creation_address` and ignores `creation_ip` 
in preparation for removing it.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Replace creation_ip with creation_address
  * Ignore the creation_ip column which is no longer used

-- File Changes --

M app/controllers/users_controller.rb (3)
M app/models/user.rb (2)
M app/views/users/_user.html.erb (4)
M app/views/users/show.html.erb (4)
M script/update-spam-blocks (2)
M test/controllers/users_controller_test.rb (2)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5218
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] Drop creation_ip column (PR #5212)

2024-09-15 Thread Tom Hughes via rails-dev
I've opened #5218 for the first phase and rebased this on that and switched it 
to draft for now.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5212#issuecomment-2351741641
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] Replace creation_ip with creation_address (PR #5218)

2024-09-15 Thread Tom Hughes via rails-dev
@tomhughes pushed 1 commit.

5bac49c51fcd8a619f0ae515ce734aa9ca31e43b  Ignore the creation_ip column which 
is no longer used

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5218/files/8bb463318d98db6c9b24e3a1d9de403e916cdf28..5bac49c51fcd8a619f0ae515ce734aa9ca31e43b
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] Order changeset elements for consistent pagination (PR #5209)

2024-09-16 Thread Tom Hughes via rails-dev
Well what I mostly want in the short term is for the tests to be reliable, but 
the reason they're not reliable is that the web site is not reliable - next and 
previous may not step through the objects reliably.

As we're paginating this data with a full count of pages we're already reading 
it all anyway so having to sort it doesn't change much.

I don't care much what the order is so long as it's stable which I'm not sure 
timestamp+version is (though adding id as a third element would guarantee 
stability).

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5209#issuecomment-2353147993
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] Drop creation_ip column (PR #5212)

2024-09-16 Thread Tom Hughes via rails-dev
This is now ready for review again, as #5218 has been merged and deployed.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5212#issuecomment-2353415803
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] Update to rails 7.2.0 (PR #5070)

2024-09-17 Thread Tom Hughes via rails-dev
@tomhughes pushed 3 commits.

6789fc30813990710c5fb786bf90a70867f77339  Drop support for ruby 3.0
be2678e0dd256de10ad176118583aab5914ef671  Update to rails 7.2.0
60c1ae79d6c56f3e6d892043dc0f9681ae8cfa92  Fix warnings about tests with no 
assertions

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5070/files/e60faba721debab4b8f58768e9ffcce126bb031f..60c1ae79d6c56f3e6d892043dc0f9681ae8cfa92
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] Update to iD v2.30.4 (PR #5250)

2024-10-08 Thread Tom Hughes via rails-dev
Merged #5250 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5250#event-14560703131
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 with Turbo Pagination and Extend to Sortable Columns (Issue #5259)

2024-10-13 Thread Tom Hughes via rails-dev
Did you close this because you realised the problem with it? That pagination 
needs any ordering to be on a unique index so that the cursor positioning can 
work...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#issuecomment-2409032242
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 with Turbo Pagination and Extend to Sortable Columns (Issue #5259)

2024-10-14 Thread Tom Hughes via rails-dev
Just to be clear my concerns are not about how the UI works, it's about the 
database queries being performed on the backend and whether they are 
safe/efficient.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#issuecomment-2410695034
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] update link for Austria data sources (Issue #5253)

2024-10-09 Thread Tom Hughes via rails-dev
As we explained in #4470 we're happy to change it if LWG are OK with that, 
given that the text as currently written is there to record a legally required 
attribution.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5253#issuecomment-2402843103
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] Update link for Austria data sources (PR #5254)

2024-10-09 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5254#issuecomment-2402979576
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] update link for Austria data sources (Issue #5253)

2024-10-09 Thread Tom Hughes via rails-dev
Yes I know, it just wasn't clear that this was an LWG decision/request given it 
said "it has been reported to" rather than saying that they had decided/agreed 
that it should be changed. I assume that was the intention though.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5253#issuecomment-2402977574
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] Update link for Austria data sources (PR #5254)

2024-10-09 Thread Tom Hughes via rails-dev
Merged #5254 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5254#event-14577286730
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] update link for Austria data sources (Issue #5253)

2024-10-09 Thread Tom Hughes via rails-dev
Closed #5253 as completed via fb73d0770b68f1b6ed50fb40e9b4cd18d9638d65.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5253#event-14577286756
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] Notes controller calls set_locale after lookup_user which can cause unexpected language for 404 users notes (Issue #5251)

2024-10-07 Thread Tom Hughes via rails-dev
We probably need to review all the `lookup_user` calls and make sure they come 
after `set_locale` and probably `web_timeout` as well though that's unlikely to 
matter much.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5251#issuecomment-2396573260
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 eslint from 9.11.1 to 9.12.0 (PR #5249)

2024-10-06 Thread Tom Hughes via rails-dev
Merged #5249 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5249#event-14531125595
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] Replace submitted note table colors with created/resolved (PR #5269)

2024-10-18 Thread Tom Hughes via rails-dev
Oh sorry I looked at the source last night and was quite sure they were all 
marked as opened.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2422681177
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] Replace submitted note table colors with created/resolved (PR #5269)

2024-10-18 Thread Tom Hughes via rails-dev
Why check every comment though when the initial open will always be the first 
one?

There's still a question about resolution I think? Should we be looking at 
every one or just the last one?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2422683789
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] Replace submitted note table colors with created/resolved (PR #5269)

2024-10-18 Thread Tom Hughes via rails-dev
After your change the test is:

```ruby
opened_by_user = note.comments.any? { |comment| comment.author == @user && 
comment.event == "opened" }
```

which will be true if the user has ever opened it, either the original open or 
a reopen?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2422575702
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] Replace submitted note table colors with created/resolved (PR #5269)

2024-10-19 Thread Tom Hughes via rails-dev
My thinking was literally just to look at the last event which should be the 
close event unless it's been reopened and if it has then do we want to consider 
it closed by this user?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2423905143
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] Use system font on error pages (PR #5277)

2024-10-21 Thread Tom Hughes via rails-dev
My PR hasn't been merged in two months so it must be good is an interesting 
take.

That said I'm not totally sure why I didn't merge it - possibly I was concerned 
about adding so much custom CSS code which we are generally trying to avoid.

Is there a reason not to use bootstrap here?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5277#issuecomment-2427791863
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] Don't show older/newer buttons if all items fit on one page (PR #5276)

2024-10-21 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5276#issuecomment-2427800288
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] Don't show older/newer buttons if all items fit on one page (PR #5276)

2024-10-21 Thread Tom Hughes via rails-dev
Merged #5276 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5276#event-14771781751
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 "User's Diary" from diary entry og:title (PR #5274)

2024-10-20 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5274#issuecomment-2425108944
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 eslint from 9.12.0 to 9.13.0 (PR #5271)

2024-10-20 Thread Tom Hughes via rails-dev
Merged #5271 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5271#event-14753418474
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 "User's Diary" from diary entry og:title (PR #5274)

2024-10-20 Thread Tom Hughes via rails-dev
Merged #5274 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5274#event-14753418472
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] Typos in docs (PR #5273)

2024-10-20 Thread Tom Hughes via rails-dev
Merged #5273 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5273#event-14753418473
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] Typos in docs (PR #5273)

2024-10-20 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5273#issuecomment-2425108351
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] Danger isn't working in CI (Issue #5267)

2024-10-21 Thread Tom Hughes via rails-dev
Incidentally the "example" in the danger repo at 
https://github.com/danger/danger/blob/master/.github/workflows/CI.yml is not 
what it seems - if you look closely it never actually runs danger, it just 
echos the command that would run it!

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2427398293
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] Show route hints on the website? (Issue #5275)

2024-10-20 Thread Tom Hughes via rails-dev
Non-mappers are not our target audience so only benefits to mappers matter here.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5275#issuecomment-2425174925
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] Avoid linking to redirects on the osmfoundation website (PR #5266)

2024-10-16 Thread Tom Hughes via rails-dev
Merged #5266 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5266#event-14685680788
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] Use unreleased version of canonical_rails to allow upgrades to rails 7.2.1+ (PR #5246)

2024-10-16 Thread Tom Hughes via rails-dev
Merged #5246 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5246#event-14686035358
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] Use unreleased version of canonical_rails to allow upgrades to rails 7.2.1+ (PR #5246)

2024-10-16 Thread Tom Hughes via rails-dev
...and now there are security alerts on rails 7.2 so I will merge this.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5246#issuecomment-2417556246
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] Added workflow for PR labeling using Danger (PR #4988)

2024-10-16 Thread Tom Hughes via rails-dev
Sadly this does not appear to be working properly ... 
https://github.com/openstreetmap/openstreetmap-website/actions/runs/11370083928/job/31629028217?pr=5266

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4988#issuecomment-2417488800
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] Avoid linking to redirects on the osmfoundation website (PR #5266)

2024-10-16 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5266#issuecomment-2417489986
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] Danger isn't working in CI (Issue #5267)

2024-10-17 Thread Tom Hughes via rails-dev
Because it serves no purpose - nothing in the danger code ever looks at that.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2421502966
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] Move api error handling and timeouts to parent class (PR #5245)

2024-10-02 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5245#issuecomment-2389242090
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] Move api error handling and timeouts to parent class (PR #5245)

2024-10-02 Thread Tom Hughes via rails-dev
Merged #5245 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5245#event-14492753539
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] Refactor api_call_* filters (Issue #4861)

2024-10-02 Thread Tom Hughes via rails-dev
Closed #4861 as completed via 83425edd8da6a01047702cbb3ac8642f3ef452fa.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/4861#event-14492753495
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] Error in bundle exec db:create (Issue #5281)

2024-10-23 Thread Tom Hughes via rails-dev
Closed #5281 as completed.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5281#event-14803320283
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] Error in bundle exec db:create (Issue #5281)

2024-10-23 Thread Tom Hughes via rails-dev
Apparently you don't have the gd2 library installed so you will need to fix 
that. I have no idea about Mac's but I believe the `brew install gd` from 
https://github.com/openstreetmap/openstreetmap-website/blob/master/INSTALL.md#macosx
 is supposed to take take of that.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5281#issuecomment-2431147203
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] Danger isn't working in CI (Issue #5267)

2024-10-22 Thread Tom Hughes via rails-dev
That's not in any way relevant - it's running in the context of the base branch 
(master) in my repo as well but the target is still there and I'm explicitly 
passing it's hash as the head.

All pull_request_target means is that it doesn't merge the PR branch into 
master before running, so that the version of danger that runs is the master 
version, but the PR branch should still be present and accessible.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2429994990
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] Danger isn't working in CI (Issue #5267)

2024-10-22 Thread Tom Hughes via rails-dev
I did some testing in my fork and I'm hopeful that 
8e54d0f2aeaad8648a60e4685fa64380c45c5631 will actually fix it.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2429930890
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] Danger isn't working in CI (Issue #5267)

2024-10-22 Thread Tom Hughes via rails-dev
It worked for 
https://github.com/tomhughes/openstreetmap-website/actions/runs/11465903348 so 
it must be something specific to cross-repo PRs :-(

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2429953036
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] Danger isn't working in CI (Issue #5267)

2024-10-22 Thread Tom Hughes via rails-dev
The token is not the problem - the problem is getting it to find the commits.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2428964609
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] Directions need to say "go straight onto road B" (Issue #5280)

2024-10-22 Thread Tom Hughes via rails-dev
Closed #5280 as not planned.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5280#event-14801809934
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] Directions need to say "go straight onto road B" (Issue #5280)

2024-10-22 Thread Tom Hughes via rails-dev
We're just relaying the instructions from the routing engines - if you have a 
problem with the instructions then you need to address it with those engines as 
we have no control over them.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5280#issuecomment-2430990488
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 with Turbo Pagination and Extend to Sortable Columns (Issue #5259)

2024-10-14 Thread Tom Hughes via rails-dev
It's only efficient if there's also an index on the columns in question and 
there's a limit to how many of those we want to add, and you also need 
uniqueness though that can be achieved by appending id to any other cursor 
field(s) to create a composite cursor.

The whole point of moving to cursor based pagination is to get away from offset 
based pagination so we definitely don't want to go back to that,

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#issuecomment-2411411260
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] Rejecting edits at the null island (0,0) (Issue #5279)

2024-10-22 Thread Tom Hughes via rails-dev
Unless it isn't like when there was a buoy moored there.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5279#issuecomment-2429403859
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] Rejecting edits at the null island (0,0) (Issue #5279)

2024-10-22 Thread Tom Hughes via rails-dev
Closed #5279 as not planned.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5279#event-14786343336
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] How to configure rapid editor in a local deployment environment? (Issue #5285)

2024-10-24 Thread Tom Hughes via rails-dev
We don't write rapid or know anything about it so we are unable to help - you 
should address your questions to the rapid developers.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5285#issuecomment-2435788665
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] How to configure rapid editor in a local deployment environment? (Issue #5285)

2024-10-24 Thread Tom Hughes via rails-dev
Closed #5285 as not planned.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5285#event-14855363967
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] Create base and head branches before running danger (PR #5295)

2024-10-29 Thread Tom Hughes via rails-dev
Let's have another go at getting danger working shall we...

This avoids the need to path danger, and resolves the problems with not 
fetching enough history for PRs with many commits, by prefetching the data and 
creating local `danger_base` and `danger_head` branches which are the default 
names that danger looks for.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Create base and head branches before running danger

-- File Changes --

M .github/workflows/danger.yml (6)
M Gemfile (2)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5295
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] Added new Danger rule for catching outdated Gemfile.lock (PR #5301)

2024-11-04 Thread Tom Hughes via rails-dev

Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5301#issuecomment-2455483008
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] Added new Danger rule for catching outdated Gemfile.lock (PR #5301)

2024-11-04 Thread Tom Hughes via rails-dev
Merged #5301 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5301#event-15109017175
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 eslint from 9.13.0 to 9.14.0 (PR #5299)

2024-11-03 Thread Tom Hughes via rails-dev
Merged #5299 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5299#event-15096130462
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] Make the "remember me" option work as intended (PR #5303)

2024-11-05 Thread Tom Hughes via rails-dev
The "remember me" checkbox has not been working properly, or rather 
it has been treated as checked even when it isn't.

The reason is the "Gotcha" details in the [check_box 
documentation](https://api.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-check_box)
 which means that the parameter has the value `"0"` when unticked.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Make the "remember me" option work as intended

-- File Changes --

M app/controllers/sessions_controller.rb (2)
M test/controllers/sessions_controller_test.rb (18)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5303
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 profile location (PR #5302)

2024-11-05 Thread Tom Hughes via rails-dev

This seems like an awful lot of complication...

It seems to me that we have the option of either allowing the user to input 
freetext, or of generating something from the location with Nominatim but this 
tries to do both at once which just makes things very complicated and probably 
quite confusing.

How often does nomination actually manage to match? I don't see any 
indication of zoom on the reverse geocode, and you're just taking the last 
name, so how likely is it that will match what the user entered even if they 
are trying to be accurate? Are we just going to wind up telling everybody their 
location doesn't match?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5302#issuecomment-2457933351
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 status filter to user's note page (PR #5297)

2024-11-05 Thread Tom Hughes via rails-dev
@tomhughes requested changes on this pull request.

In addition to the inline comment this could do with tests at the controller or 
system level rather than just testing the model method.

> @@ -34,6 +34,16 @@ class Note < ApplicationRecord
 
   scope :visible, -> { where.not(:status => "hidden") }
   scope :invisible, -> { where(:status => "hidden") }
+  scope :with_status, lambda { |status|
+case status
+when "open"
+  where(:status => "open")
+when "closed"
+  where(:status => "closed")
+else
+  all
+end
+  }

Rather than trying to handle the all case specially here I think I would make 
this much simpler, like the `with_status` on the issue model, and then add an 
`unless params[:status] == "all"` guard in the controller so that the filter 
isn't applied when all statuses are requested.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297#pullrequestreview-2416462718
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] Create base and head branches before running danger (PR #5295)

2024-10-30 Thread Tom Hughes via rails-dev
@tomhughes pushed 1 commit.

356738e5c502b6907864ac1f0fed235780c0  Create base and head branches before 
running danger

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5295/files/c4be01e3fb7a32342ee449816ba3f5e7baaa8009..356738e5c502b6907864ac1f0fed235780c0
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] Note subscriptions db table (PR #5284)

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



> @@ -401,6 +401,8 @@ def add_comment(note, text, event, notify: true)
   note.comments.map(&:author).uniq.each do |user|
 UserMailer.note_comment_notification(comment, user).deliver_later if 
notify && user && user != current_user && user.visible?
   end
+
+  NoteSubscription.find_or_create_by(:note => note, :user => current_user) 
if current_user

Hmm you're quite right, and rubocop is quite right, which I find quite odd but 
apparently ruby does continue evaluating the line after the first `&.` fails.

Personally I still think I prefer it but it's not a huge issue.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5284#discussion_r1823215317
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] Create base and head branches before running danger (PR #5295)

2024-10-30 Thread Tom Hughes via rails-dev
That should be fixed now...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5295#issuecomment-2447468574
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 leaflet.locatecontrol from 0.81.1 to 0.82.0 (PR #5296)

2024-10-30 Thread Tom Hughes via rails-dev
Merged #5296 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5296#event-15014807245
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] Note subscriptions db table (PR #5284)

2024-10-31 Thread Tom Hughes via rails-dev

It looks like there are no significant issues here, so I'll merge this. 
Thanks for the work.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5284#issuecomment-2450517476
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] Note subscriptions db table (PR #5284)

2024-10-31 Thread Tom Hughes via rails-dev
Merged #5284 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5284#event-15043488192
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] Unauthenticated session expiry (PR #5270)

2024-10-28 Thread Tom Hughes via rails-dev
Not sure why you closed this? I actually spent some time yesterday digging into 
it... 

I am interested in knowing if you've found any sources backing up the theory 
that entries with no expiry can somehow swamp out ones with an expiry as I'm 
not really clear on how that would happen?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5270#issuecomment-2442395696
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] Note subscriptions db table (PR #5284)

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



> @@ -401,6 +401,8 @@ def add_comment(note, text, event, notify: true)
   note.comments.map(&:author).uniq.each do |user|
 UserMailer.note_comment_notification(comment, user).deliver_later if 
notify && user && user != current_user && user.visible?
   end
+
+  NoteSubscription.find_or_create_by(:note => note, :user => current_user) 
if current_user

I don't believe it does - if `current_user` is defined then the 
`note_subscriptions` association will always exist. It might evaluate to an 
empty list but the association object will exist and can have methods invoked 
on it.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5284#discussion_r1818219516
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] Consistent use of quotes (PR #5291)

2024-10-27 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5291#issuecomment-2439937447
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 articles before "exit" and "roundabout" (PR #5292)

2024-10-27 Thread Tom Hughes via rails-dev
Merged #5292 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5292#event-14922059585
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] Consistent use of quotes (PR #5291)

2024-10-27 Thread Tom Hughes via rails-dev
Merged #5291 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5291#event-14922059588
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 articles before "exit" and "roundabout" (PR #5292)

2024-10-27 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5292#issuecomment-2439937586
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] Danger isn't working in CI (Issue #5267)

2024-10-27 Thread Tom Hughes via rails-dev
Well that's interesting because 
https://github.com/danger/danger/blob/53ebd6415175ac7611b8605d5c8d20905268404c/lib/danger/scm_source/git_repo.rb#L85-L91
 is the key code and that is supposed to try four passes.

The first pass looks at a depth of 20 but if that fails it should retry to 74, 
222 and 625 before giving up...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2440151129
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] Note subscriptions db table (PR #5284)

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



> @@ -401,6 +401,8 @@ def add_comment(note, text, event, notify: true)
   note.comments.map(&:author).uniq.each do |user|
 UserMailer.note_comment_notification(comment, user).deliver_later if 
notify && user && user != current_user && user.visible?
   end
+
+  NoteSubscription.find_or_create_by(:note => note, :user => current_user) 
if current_user

I think this could be simplified using the association?

```suggestion
  current_user&.note__subscriptions.find_or_create_by(:note => note)
```

> +end
+  end
+  assert_response :success
+  js = ActiveSupport::JSON.decode(@response.body)
+  assert_not_nil js
+  assert_equal "Feature", js["type"]
+  assert_equal "Point", js["geometry"]["type"]
+  assert_equal [-1.0, -1.0], js["geometry"]["coordinates"]
+  assert_equal "open", js["properties"]["status"]
+  assert_equal 1, js["properties"]["comments"].count
+  assert_equal "opened", js["properties"]["comments"].last["action"]
+  assert_equal "This is a comment", 
js["properties"]["comments"].last["text"]
+  assert_equal user.display_name, js["properties"]["comments"].last["user"]
+
+  note = Note.last
+  subscription = NoteSubscription.last

This (and all the other similar versions in other tests) is relying on nothing 
else having created notes or users in a way that overlaps this test and I'm not 
sure how safe that is when tests are running in parallel?

Maybe it would be better to get the node ID from the JSON response and then do 
`assert Note.exists?(id)` and a similar exists assertion on the subscription 
record?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5284#pullrequestreview-2397146273
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.3 to 2.3.4 (PR #5286)

2024-10-26 Thread Tom Hughes via rails-dev
Merged #5286 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5286#event-14900309148
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 callback from initialize() in note js controller (PR #5289)

2024-10-26 Thread Tom Hughes via rails-dev
Merged #5289 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5289#event-14900309139
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] Improvements to danger workflow (PR #5290)

2024-10-26 Thread Tom Hughes via rails-dev
Ah sorry... I cherry picked and forgot I had updated the danger branch since. 
Now fixed.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5290#issuecomment-2439549811
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 callback from initialize() in note js controller (PR #5289)

2024-10-26 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5289#issuecomment-2439476251
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] Decrease padding between close button and inputs in directions form (PR #5288)

2024-10-26 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5288#issuecomment-2439475223
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] Danger isn't working in CI (Issue #5267)

2024-10-24 Thread Tom Hughes via rails-dev
I've opened https://github.com/danger/danger/pull/1501 for upstream that I 
believe should fix things...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5267#issuecomment-2436035422
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] How to configure rapid editor in a local deployment environment? (Issue #5285)

2024-10-24 Thread Tom Hughes via rails-dev
The only thing I would say (and it won't be the cause of the problem) is that 
you don't need to put the rapid id or keys in `settings.local.yml` as the rails 
code doesn't need them and by doing so you're duplicating those settings and 
overwriting the id and key the web site uses for itself.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5285#issuecomment-2435794200
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] Decrease padding between close button and inputs in directions form (PR #5288)

2024-10-26 Thread Tom Hughes via rails-dev
Merged #5288 into master.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5288#event-14900309160
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] Improvements to danger workflow (PR #5290)

2024-10-26 Thread Tom Hughes via rails-dev
@tomhughes pushed 3 commits.

754e811b70aa309b080e102ed629672f1558866b  Get danger working in GitHub actions
8e566b0bff66e23389a6fb1c7458696c8e06faf1  Avoid suggesting danger is only about 
labels
67749e75347ee7ae94ba6f6929846389d8e12118  Make danger labels use consisent 
capitalisation

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5290/files/f6c2e43bcfe79d6b86986d374f4d83dcf950daf0..67749e75347ee7ae94ba6f6929846389d8e12118
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 articles before "exit" and "roundabout" (PR #5292)

2024-10-26 Thread Tom Hughes via rails-dev
You don't need to close the PR and open a new one to make changes - just update 
your branch and force push it and the PR will pick up the changes,

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5292#issuecomment-2439625134
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] Improvements to danger workflow (PR #5290)

2024-10-26 Thread Tom Hughes via rails-dev
This gets the danger workflow working by switching to my fixed version (current 
open upstream as https://github.com/danger/danger/pull/1501) and also tweaks a 
few other things:

* Renames the workflow to reflect that this is not just about labelling
* Drops the `run-name` so that he runs will be names after the PR instead of 
all having the same name
* Adjusts the label names to be consistent with our other labels.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Get danger working in GitHub actions
  * Avoid suggesting danger is only about labels
  * Make danger labels use consisent capitalisation

-- File Changes --

R .github/workflows/danger.yml (4)
M Dangerfile (12)
M Gemfile (2)
M Gemfile.lock (36)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5290
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] Restore loading spinner in richtext previews (PR #5311)

2024-11-12 Thread Tom Hughes via rails-dev
Looks good to me, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5311#issuecomment-2471323757
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] Make the "remember me" option work as intended (PR #5303)

2024-11-13 Thread Tom Hughes via rails-dev
It needed a slight tweak to the view because when the form is rerendered on a 
failed login the parameter on the redirect comes from the session so now has a 
different value. It should be fixed now.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5303#issuecomment-2473463415
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] Better database discovery (PR #5308)

2024-11-13 Thread Tom Hughes via rails-dev
There's a button or something somewhere you can click to turn on the 
automatically detected relationships but I admit it confused me as well.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5308#issuecomment-2473466329
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] Make the "remember me" option work as intended (PR #5303)

2024-11-13 Thread Tom Hughes via rails-dev
@tomhughes pushed 1 commit.

a42b6546060227fce391c54557d7383f0291f410  Make the "remember me" option work as 
intended

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5303/files/035e996ab9609c3828bb5c1b1552c42e27fcd662..a42b6546060227fce391c54557d7383f0291f410
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] Switch vagrant to use Debian 12 (PR #5158)

2024-11-13 Thread Tom Hughes via rails-dev
@tomhughes pushed 1 commit.

5e26b33235bab027f25f98afb8db8980f3603b0b  Switch vagrant to use Debian 12

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5158/files/1cf6f65a5eec461ebe268d7141e5c5cd444eb02f..5e26b33235bab027f25f98afb8db8980f3603b0b
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


  1   2   3   4   5   6   7   8   9   10   >