Re: [openstreetmap/openstreetmap-website] Add social sharing functionality (PR #4985)

2024-09-16 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

4efc63d6090eef1bb334fa7b58c123cf4d80577b  Add social sharing functionality

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985/files/ff4b223f68baf21e70e79acc5f82eca34b872ab2..4efc63d6090eef1bb334fa7b58c123cf4d80577b
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 social sharing functionality (PR #4985)

2024-09-16 Thread Emin Kocan via rails-dev
As @AntonKhorev suggested, we're now displaying these icons only when users 
open the full diary entry, as outlined in #5121. Since this is currently used 
in one place, turbo pagination issues can be handled in a future PR if a use 
case arises for combining it with turbo pagination. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#issuecomment-2354636832
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 Emin Kocan via rails-dev
Closed #5259 as not planned.

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

2024-10-13 Thread Emin Kocan via rails-dev
### Problem

Currently, we use a custom `PaginationMethods` module, which sorts and filters 
data using IDs. Sorting by other attributes (e.g., created_at or updated_at) 
does not work correctly, and implementing sorting with these attributes would 
require changes to `PaginationMethods`. Additionally, the Notes page does not 
currently use Turbo, but refactoring it to support sorting with pagination 
would allow seamless integration of this feature.

### Description

`PaginationMethods` could be modified module to handle sorting on attributes 
other than IDs, integrated with Turbo for pagination. This would allow us to 
add sorting to tables across the site. As an example, this feature could later 
be extended to the Notes page, where adding Turbo pagination would enhance the 
user experience with sortable columns and icons next to headers.

I am eager to do some research and explore the best approach for this. Would 
appreciate any suggestions or ideas on how to proceed.

### Screenshots

On UI end integrating this into tables could look something like this(we would 
just add the sorting icons):
https://github.com/user-attachments/assets/ca4cddfc-8060-437a-b49a-4eebdb56b554";>


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

7b7e6550ce7ccac73c075257cd71807858cb364d  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/91e8d556fe976b42a4aabfc594ac905304d37d3d..7b7e6550ce7ccac73c075257cd71807858cb364d
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

bc453f0bb14709adc3997f371c8e6121e7f6e3b3  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/7b7e6550ce7ccac73c075257cd71807858cb364d..bc453f0bb14709adc3997f371c8e6121e7f6e3b3
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 7 commits.

9266180344761718d4068dd62f78ef3a0169b8e1  Use teaspoon to run javascript tests
1c47363ae310ddf11760e64263ca763077a6d3bf  Run javascript tests in CI
123c7d10a5e798e2379dfb04df4a22b1deface3c  Selenium driver working on local
5c7b7383e71380d4189385c6d05430649e0e7496  Use chai assertion style
b17517e02cc9580b89a220d926292166aed25b9b  Disable eslint for js test files to 
test for github-ci
e5c5776e65f1a461d7d1aa678e843c9cf83316c2  Add step for creation tmp/pids in 
test workflow
6a3a0e5c02a4e0c54d93414257b8e03fce33eb91  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/97540a304302822d27dffde3d0c843c616a65583..6a3a0e5c02a4e0c54d93414257b8e03fce33eb91
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

40c71f28be97ffcafb47058fd2590020fb8567a1  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/6a3a0e5c02a4e0c54d93414257b8e03fce33eb91..40c71f28be97ffcafb47058fd2590020fb8567a1
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -9,6 +9,13 @@ SET xmloption = content;
 SET client_min_messages = warning;
 SET row_security = off;
 
+--
+-- Name: public; Type: SCHEMA; Schema: -; Owner: -
+--
+
+-- *not* creating schema, since initdb creates it
+
+

Removed changes to this file in the PR. Thank you

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216#discussion_r1797748924
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
> I suppose the issues listed below are a result of the changes in #5084. I 
> would simply fix the respective unit tests to re-align them with the current 
> implementation.
> 
> ```
>   1) OSM .formatHash formats lat/lon/zoom params
>  Failure/Error: expected '#map=9/57.625/-3.685' to equal 
> '#map=9/57.6247/-3.6845'
> 
>   2) OSM .formatHash respects zoomPrecision
>  Failure/Error: expected '#map=5/57.62/-3.68' to equal 
> '#map=5/57.625/-3.685'
> 
>   3) OSM .formatHash formats layers params
>  Failure/Error: expected '#map=9/57.625/-3.685&layers=C' to equal 
> '#map=9/57.6247/-3.6845&layers=C'
> 
>   4) OSM .formatHash ignores default layers
>  Failure/Error: expected '#map=9/57.625/-3.685' to equal 
> '#map=9/57.6247/-3.6845'
> 
>   5) OSM .zoomPrecision suggests 0 digits for z0-1
>  Failure/Error: expected 1 to equal 0
> 
>   6) OSM .zoomPrecision suggests 3 digits for z5-8
>  Failure/Error: expected 2 to equal 3
> 
>   7) OSM .zoomPrecision suggests 4 digits for z9-16
>  Failure/Error: expected 3 to equal 4
> 
>   8) OSM .zoomPrecision suggests 5 digits for z17-20
>  Failure/Error: expected 6 to equal 5
> 
>   9) OSM .locationCookie creates a location cookie value
>  Failure/Error: expected '-3.685|57.625|9|M' to equal 
> '-3.6845|57.6247|9|M'
> 
>   10) OSM .locationCookie respects zoomPrecision
>  Failure/Error: expected '-3.685|57.625|9|M' to equal 
> '-3.6845|57.6247|9|M'
> ```

Thank you for linking the PR so I could update the tests according to new 
implementation. All tests should be passing now. I also added running 
javascripts tests command to `CONTRIBUTING.md`.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216#issuecomment-2408656554
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -144,6 +144,9 @@ gem "unicode-display_width"
 # Keep ruby 3.0 compatibility
 gem "multi_xml", "~> 0.6.0"
 
+gem "teaspoon"
+gem "teaspoon-mocha", "~> 2.3.3"

Moved the gems to `group :development, :test` which made a bit more sense to me 
since running tests via command line is a bit more straight forward. Thank you 
for review!

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216#discussion_r1797750335
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

9ca88734655f1836fcdc6455cdcdb9dba2535ec4  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/717a8dd4a4193c07a828dc1b92fc7020a0730006..9ca88734655f1836fcdc6455cdcdb9dba2535ec4
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

2306d56b1daeffa2dbecfed6adfff5e5378c4d5f  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/9ca88734655f1836fcdc6455cdcdb9dba2535ec4..2306d56b1daeffa2dbecfed6adfff5e5378c4d5f
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] Re-enable JavaScript unit tests using Teaspoon (PR #5216)

2024-10-12 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

97540a304302822d27dffde3d0c843c616a65583  Enable eslint and fix eslint errors 
for osm_test.js

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5216/files/2306d56b1daeffa2dbecfed6adfff5e5378c4d5f..97540a304302822d27dffde3d0c843c616a65583
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] Sorting and filtering notes on the profile[TEST RUN] (PR #5255)

2024-10-09 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

e382dfa8eb4cfbf63d93645542142b98cb19f149  Add tests to note controller and 
handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/63ca32725b6f808e2e5288573643facfcf10300e..e382dfa8eb4cfbf63d93645542142b98cb19f149
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] Sorting and filtering notes on the profile[TEST RUN] (PR #5255)

2024-10-09 Thread Emin Kocan via rails-dev

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

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

-- Commit Summary --

  * Add filter and sort scopes to note model
  * Add tests for filtering and sort scopes inside note model
  * Integrate filtering and sorting methods into notes controller
  * Add sorting and filtering functionality to note view
  * Add test to controller and update scopes and filtering to handle edge cases

-- File Changes --

M app/controllers/notes_controller.rb (19)
M app/models/note.rb (48)
M app/views/notes/index.html.erb (32)
M config/locales/en.yml (13)
A package-lock.json (1378)
M test/controllers/notes_controller_test.rb (135)
M test/models/note_test.rb (140)
M yarn.lock (321)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255
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] Sorting and filtering notes on the profile[TEST RUN] (PR #5255)

2024-10-09 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

2e18a7cc7552c729dcc147e1391e286fca4d  Add test to controller and update 
scopes and filtering to handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/d8d40c10734f3012f854e8eedadc8ba92ee74656..2e18a7cc7552c729dcc147e1391e286fca4d
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] Sorting and filtering notes on the profile[TEST RUN] (PR #5255)

2024-10-09 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

63ca32725b6f808e2e5288573643facfcf10300e  Add test to controller and update 
scopes and filtering to handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/2e18a7cc7552c729dcc147e1391e286fca4d..63ca32725b6f808e2e5288573643facfcf10300e
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] Sorting and filtering notes on the profile[TEST RUN] (PR #5255)

2024-10-09 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

721f8d52810673783b48341ecfbb9b7305ae4134  Add tests to note controller and 
handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/e382dfa8eb4cfbf63d93645542142b98cb19f149..721f8d52810673783b48341ecfbb9b7305ae4134
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-16 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

d88ec3046dd0257e856010720417226ffe64e8aa  Add tests to note controller and 
handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/d87609f00fd6ed04514f08f0dde2ec7279d8df30..d88ec3046dd0257e856010720417226ffe64e8aa
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-16 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -27,19 +27,19 @@
 
 
 
-  <%= t ".my friends" %>
+  <%= t ".followed users" %>

Maybe use "Following" instead of "Followed users" for better clarity. It aligns 
with the common terminology users are familiar with, like "Following" and 
"Followers," which may feel more intuitive. Just a thought.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5261#pullrequestreview-2372872353
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-16 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

84a01a3174d1383e85584f0d6689d4e07a4b6bed  Use turbo for filtering on select 
without refreshing the whole page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/d88ec3046dd0257e856010720417226ffe64e8aa..84a01a3174d1383e85584f0d6689d4e07a4b6bed
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-16 Thread Emin Kocan via rails-dev
Thank you for the detailed background and context, Anton. I agree that using 
date pickers makes sense if we’re filtering notes by dates, especially since 
the current setup isn’t relying on standard pagination. 

As for the next steps, I’m not sure whether it’s better to get this PR merged 
as is and refactor later when the notes move to standard pagination, or if we 
should address the pagination issue first and then refactor this PR accordingly.

On a side note, I’ve managed to implement sortable columns for `created_at` and 
`updated_at` locally, but I’ll hold off on submitting that until this PR is 
merged to keep things cleaner and easier to review.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2417243495
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-16 Thread Emin Kocan 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 - 
> [Add oldest/newest links to shared pagination 
> #4710](https://github.com/openstreetmap/openstreetmap-website/pull/4710) [Add 
> oldest/newest links to shared pagination - with gap between "newer" and 
> "older" 
> #4729](https://github.com/openstreetmap/openstreetmap-website/pull/4729) [Add 
> oldest/newest links to shared pagination - with gaps around "newer" and 
> "older" 
> #4733](https://github.com/openstreetmap/openstreetmap-website/pull/4733) [Add 
> oldest/newest links to shared pagination - with gaps around "newer" and 
> "older" v2 
> #4734](https://github.com/openstreetmap/openstreetmap-website/pull/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 [Convert note pagination to newer/older 
> note pages 
> #4532](https://github.com/openstreetmap/openstreetmap-website/pull/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.

Thank you for the detailed background and context, Anton. I agree that using 
date pickers makes sense if we’re filtering notes by dates, especially since 
the current setup isn’t relying on standard pagination. 

As for the next steps, I’m not sure whether it’s better to get this PR merged 
as is and refactor later when the notes move to standard pagination, or if we 
should address the pagination issue first and then refactor this PR accordingly.

On a side note, I’ve managed to implement sortable columns for `created_at` and 
`updated_at` locally, but I’ll hold off on submitting that until this PR is 
merged to keep things cleaner and easier to review.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2417241994
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-16 Thread Emin Kocan via rails-dev
@kcne 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"),

Thank you for pointing that out. I've remove them and added preselected all 
fields accordingly.


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#discussion_r1803075462
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-16 Thread Emin Kocan via rails-dev
@kcne 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") %>

You're right. I've renamed that into interaction type which is a better naming 
for this.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#discussion_r1803076747
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-16 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

7851e30c263079c28a13e18005339c1cdf00e537  Add tests to note controller and 
handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/721f8d52810673783b48341ecfbb9b7305ae4134..7851e30c263079c28a13e18005339c1cdf00e537
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-16 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



>  
-friends_heading = find :element, "h2", :text => "My friends"
+friends_heading = find :element, "h2", :text => "Followed users"

Maybe use  "Following" instead of "Followed users" for better clarity. It 
aligns with the common terminology users are familiar with, like "Following" 
and "Followers," which may feel more intuitive.

> -  heading: "Unfriend %{user}?"
-  button: "Unfriend"
-  success: "%{name} was removed from your friends."
-  not_a_friend: "%{name} is not one of your friends."
+follow_user:
+  heading: "Do you want to follow %{user}?"
+  button: "Follow User"
+  success: "You are now following %{name}!"
+  failed: "Sorry, failed to follow %{name}."
+  already_followed: "You already follow %{name}."
+  limit_exceeded: "You have followed a lot of users recently. Please wait 
a while before trying to follow any more."
+unfollow_user:
+  heading: "Do you want to unfollow %{user}?"
+  button: "Unfollow"
+  success: "%{name} was unfollowed by you."
+  not_followed: "%{name} is not followed by you."

A bit more active voicing alternative would be "You are not following %{name}".

> -remove_friend:
-  heading: "Unfriend %{user}?"
-  button: "Unfriend"
-  success: "%{name} was removed from your friends."
-  not_a_friend: "%{name} is not one of your friends."
+follow_user:
+  heading: "Do you want to follow %{user}?"
+  button: "Follow User"
+  success: "You are now following %{name}!"
+  failed: "Sorry, failed to follow %{name}."
+  already_followed: "You already follow %{name}."
+  limit_exceeded: "You have followed a lot of users recently. Please wait 
a while before trying to follow any more."
+unfollow_user:
+  heading: "Do you want to unfollow %{user}?"
+  button: "Unfollow"
+  success: "%{name} was unfollowed by you."

This sound a bit passive to me, altough I'm not sure. Alternative could be 
something like "You successfully unfollowed %{name}."

> -  heading: "Add %{user} as a friend?"
-  button: "Add as friend"
-  success: "%{name} is now your friend!"
-  failed: "Sorry, failed to add %{name} as a friend."
-  already_a_friend: "You are already friends with %{name}."
-  limit_exceeded: "You have friended a lot of users recently. Please wait 
a while before trying to friend any more."
-remove_friend:
-  heading: "Unfriend %{user}?"
-  button: "Unfriend"
-  success: "%{name} was removed from your friends."
-  not_a_friend: "%{name} is not one of your friends."
+follow_user:
+  heading: "Do you want to follow %{user}?"
+  button: "Follow User"
+  success: "You are now following %{name}!"
+  failed: "Sorry, failed to follow %{name}."

Suggesting one more alternative: "Sorry, your request for following %{name} has 
failed." Wording sounds a bit unnatural here imho

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5261#pullrequestreview-2372819677
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-16 Thread Emin Kocan via rails-dev
@kcne pushed 6 commits.

c49aaf4b0256f0c1904b16ec7a71a7176c36b7b4  Add filter and sort scopes to note 
model
0117a279264b962663c529c5cb49b14aa2933e4a  Add tests for filtering and sort 
scopes inside note model
0f1045e9480989d54c2fe6fa7fa37fad1adbfd3f  Integrate filtering and sorting 
methods into notes controller
f5394d2d62e0bd461976e56f2149aeb646f0843e  Add sorting and filtering 
functionality to note view
bafa9be190fdfcf5c7db118899a3cf46d58487e3  Add tests to note controller and 
handle edge cases
8725d95265e95b1b16610bbc6d483a0afffe9d9e  Use turbo for filtering on select 
without refreshing the whole page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/84a01a3174d1383e85584f0d6689d4e07a4b6bed..8725d95265e95b1b16610bbc6d483a0afffe9d9e
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-16 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

d87609f00fd6ed04514f08f0dde2ec7279d8df30  Add tests to note controller and 
handle edge cases

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/7851e30c263079c28a13e18005339c1cdf00e537..d87609f00fd6ed04514f08f0dde2ec7279d8df30
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-21 Thread Emin Kocan via rails-dev
> Besides the [double-join db 
> query](https://github.com/openstreetmap/openstreetmap-website/pull/5255#discussion_r1801064970),
>  there's couple more issues with the interaction type: ["blue on blue" 
> colors](https://github.com/openstreetmap/openstreetmap-website/pull/4700#issuecomment-2221682594)
>  and whether the submitted/commented classification makes sense at all (what 
> if I closed a note without a comment?). I wanted to try 
> opened/closed/commented instead.

First of all, thank you for taking time and effort reviewing this. Few things I 
would like to reassess:
 - Related to the commented/submitted criteria we could use `EXISTS` subqueries 
which removes the need for a double joins, potentially making the query simpler 
and more performant. I could try testing this in terms of performance with 
large number of mocked entries so we can better get the idea of how this would 
affect the performance.

```ruby
  scope :filter_by_note_type, lambda { |note_type, user_id|
case note_type
when "commented"
  where("EXISTS (SELECT 1 FROM note_comments WHERE note_comments.note_id = 
notes.id AND (note_comments.author_id != ? OR note_comments.author_id IS 
NULL))", user_id)
when "submitted"
  where("EXISTS (SELECT 1 FROM note_comments WHERE note_comments.note_id = 
notes.id AND note_comments.author_id = ? AND note_comments.id = (SELECT 
MIN(nc.id) FROM note_comments nc WHERE nc.note_id = notes.id))", user_id)
else
  all
end
  }
```

- Regarding the open/closed/commented differentiation instead of 
commented/submitted - It makes sense to me too but I would also like to hear 
the @tomhughes @gravitystorm opinions on this too, so we could all sync on 
right approach for this.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2426960898
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-21 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -6,41 +7,69 @@
:commented => tag.span(t(".subheading_commented"), :class => "px-2 
py-1 bg-body") %>
 <% end %>
 
-<% if @notes.empty? %>
-  <%= t ".no_notes" %>
+<%= 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] || "all"),
+  :class => "form-select",
+  :onchange => "this.form.requestSubmit()" %>

Locally, this works as expected for me. Could you provide more details on how 
to reproduce this issue locally, or is this something that only occurs on 
production servers? Thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#discussion_r1809017667
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 PaginationMethods to Support Sorting with Turbo Integration (Issue #5259)

2024-10-21 Thread Emin Kocan via rails-dev
Closed #5259 as completed.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#event-14770454975
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 social sharing functionality (PR #4985)

2024-10-21 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+  if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console 
warnings should be avoided.
+return;
+  }
+
+  const width = Math.max(100, Math.min(screen.width, initialWidth));
+  const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+  const left = (screen.width / 2) - (width / 2);
+  const top = (screen.height * 0.3) - (height / 2);
+  const opts = 
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+  window.open(url, "popup", opts);

Expected behaviour is to open mailto link in new tab where you can select your 
mail app/provider, which should be working for the timebeing. Do you suggest we 
should handle mail sharing differently?


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1809424171
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 Emin Kocan via rails-dev
@kcne approved this pull request.





-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5277#pullrequestreview-2383177034
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 Emin Kocan via rails-dev
Tom, I want to dig a bit deeper into this to explore limitations and what are 
some solid ways to approach this, after which I will reopen this again. I've 
read few blog posts where people use Turbo with 
[Stimulus](https://github.com/hotwired/stimulus) but I'm not sure if that would 
fit the bill for our use case.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#issuecomment-2410669113
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 Emin Kocan via rails-dev
You bring up a valid point about the limitations of cursor-based pagination 
with sorting. Cursor pagination works efficiently when sorting by a unique or 
sequential field (like `id`, `created_at`, or `updated_at`), but it breaks down 
when we try to sort by non-unique columns (e.g., `name`, `status`).

To address this, there are a couple of potential solutions:

1. **Combined Sorting**: We could continue using cursor-based pagination but 
with a fallback to a secondary unique column like `id`. This way, sorting by 
`name` or `status` would still function but would maintain stable pagination by 
also ordering by `id` to ensure uniqueness:
   
   ```sql
   ORDER BY name ASC, id ASC
   ```

   This approach would retain the benefits of cursor-based pagination while 
allowing some flexibility with sorting.

2. **Offset-based Pagination**: If we want to fully support sorting by 
arbitrary columns, we could switch to offset-based pagination when sorting by 
non-unique columns. This is less efficient on larger datasets but avoids the 
problem of ambiguous cursor positions.

3. **Hybrid Approach**: Implement cursor-based pagination for sortable fields 
that are unique (like `id` or timestamps), and fallback to offset-based 
pagination for others.

Before proceeding, I’d like to get some thoughts on which approach fits best 
with our performance requirements and use cases. If we'd prefer sticking with 
cursor-based pagination, I can explore implementing the combined sorting 
method, as it strikes a balance between efficiency and flexibility.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#issuecomment-2411376926
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 Emin Kocan via rails-dev
Reopened #5259.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5259#event-14636898955
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 social sharing functionality (PR #4985)

2024-10-22 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+  if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console 
warnings should be avoided.
+return;
+  }
+
+  const width = Math.max(100, Math.min(screen.width, initialWidth));
+  const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+  const left = (screen.width / 2) - (width / 2);
+  const top = (screen.height * 0.3) - (height / 2);
+  const opts = 
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+  window.open(url, "popup", opts);

Actually for me it works like this - I'm using firefox, hadn't check for the 
other browsers.

When I click share it opens share link in pop up window to select the email 
provider:
https://github.com/user-attachments/assets/d7eee376-638c-4684-ae79-96efecffd0b5";>
After email provider selection, i get the automatically formatted email message 
ready to be sent:
https://github.com/user-attachments/assets/804f34bb-2ef3-43af-a615-df2b5bebd23f";>



-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1810192445
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 social sharing functionality (PR #4985)

2024-10-22 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+  if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console 
warnings should be avoided.
+return;
+  }
+
+  const width = Math.max(100, Math.min(screen.width, initialWidth));
+  const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+  const left = (screen.width / 2) - (width / 2);
+  const top = (screen.height * 0.3) - (height / 2);
+  const opts = 
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+  window.open(url, "popup", opts);

If you have some suggestions on alternative approach I'm all ears..

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1810193904
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 filtering functionality to user notes page (PR #5255)

2024-10-25 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

ec7668874ba6da72d718661746baf5460c37012e  Remove filtering by interaction. 
Group status by open, closed and commented

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/900b9153f3daecf2bc8bcf3e10ac6a5cfc380a8d..ec7668874ba6da72d718661746baf5460c37012e
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 filtering functionality to user notes page (PR #5255)

2024-10-24 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

cc6c281028aef5983500b62091321f4b215bba3c  Remove filtering by interaction. 
Group status by open, closed and commented

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/8725d95265e95b1b16610bbc6d483a0afffe9d9e..cc6c281028aef5983500b62091321f4b215bba3c
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 filtering functionality to user notes page (PR #5255)

2024-10-24 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

900b9153f3daecf2bc8bcf3e10ac6a5cfc380a8d  Remove filtering by interaction. 
Group status by open, closed and commented

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255/files/cc6c281028aef5983500b62091321f4b215bba3c..900b9153f3daecf2bc8bcf3e10ac6a5cfc380a8d
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 filtering functionality to user notes page (PR #5255)

2024-10-24 Thread Emin Kocan via rails-dev
> Time for what? For PR review?

Exactly, since each PR would need to be reviewed sequentially, it would prolong 
the overall process.

> Are you sure? Is it complete with exactly the filters you added and not some 
> other filters?

Yes, the goal was to implement the filters that were most commonly requested 
and improve the feature based on that feedback. However, if you have any 
suggestions for additional filters that could enhance the usability for 
mappers, I’m definitely open to discussing and considering those ideas too.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2435315280
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 social sharing functionality (PR #4985)

2024-10-28 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

8ba2316df11bc0de908f589c4ff78a0337726531  Add social sharing functionality

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985/files/4efc63d6090eef1bb334fa7b58c123cf4d80577b..8ba2316df11bc0de908f589c4ff78a0337726531
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 social sharing functionality (PR #4985)

2024-10-28 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+  if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console 
warnings should be avoided.
+return;
+  }
+
+  const width = Math.max(100, Math.min(screen.width, initialWidth));
+  const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+  const left = (screen.width / 2) - (width / 2);
+  const top = (screen.height * 0.3) - (height / 2);
+  const opts = 
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+  window.open(url, "popup", opts);

In this case the email dialog gets opened right away. Fixed now, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1820170671
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] Build a mechanism for notifications on the web site, not using email (#1387)

2024-10-28 Thread Emin Kocan via rails-dev
After a bit of research on some libraries that offer such functionalities and 
implementation details that we could potentially use for this:
### 1. [notifications-rails](https://github.com/jonhue/notifications-rails)
`notifications-rails` provides a modular structure for creating, rendering, 
pushing, and managing settings for notifications, with each module backed by 
the database. This setup supports on-site notifications with user-configurable 
preferences, allowing users to see unread counts and mark notifications as read.

**Database Structure and Requirements:**

- **New Tables:**
  - **`notifications`:** Main table storing each notification. Key columns:
- `target_id` and `target_type` (polymorphic association for the recipient)
- `object_id` and `object_type` (polymorphic link to the notification’s 
subject, e.g., a `Comment`)
- `metadata` (JSON for additional data), `category` (notification type), 
and `read` (boolean for read/unread).
  - **`notification_groups` (optional):** Allows bulk notifications for groups 
(e.g., subscribers).

- **Modified Tables:**
  - **User Table**: Adds optional columns for `read_notification_count` and 
`unread_notification_count` to cache unread counts, and `settings` for storing 
user-specific notification preferences.

- **Indexes:**  
  - Indexes on `target_id`, `target_type`, and `category` for quick lookups in 
the `notifications` table.

**Database Workflow:**
1. **Creating Notifications:** Notifications are inserted into `notifications` 
with details on target, subject, metadata, and category.
2. **Marking as Read:** Updating `read` marks notifications as read, and 
`unread_notification_count` can be adjusted for caching.
3. **Bulk Notifications:** The `notification_groups` table can manage batch 
notifications by defining recipient groups.

### 2. [noticed](https://github.com/excid3/noticed)

`noticed` offers individual and bulk notifications, supporting multiple 
delivery methods, such as ActionCable for real-time updates. It’s suitable for 
creating on-site notifications tied to specific events, each managed through 
ActiveJob for asynchronous processing.

**Database Structure and Requirements:**

- **New Tables:**
  - **`notified_events`:** Logs each event that triggers notifications. Key 
columns:
- `type` (event type, e.g., “NewCommentNotifier”)
- `record_id` and `record_type` (polymorphic association to link with the 
event’s subject, e.g., `Post`)
- `params` (JSON to store event details, like message and URL).
  
  - **`notifications`:** Links notifications to events and tracks each 
recipient. Key columns:
- `event_id` (foreign key to `notified_events`), `recipient_id` and 
`recipient_type` (polymorphic association for the recipient), and `read_at` 
(datetime for read status).

- **Modified Tables:**
  - **User Table**: No required modifications, but adding `has_many 
:notifications` makes it easier to manage notifications.

- **Indexes:**
  - Indexes on `type`, `record_type`, and `record_id` in `notified_events` for 
efficient filtering and quick retrieval.

**Database Workflow:**
1. **Logging Events:** Each new event (e.g., a comment) is logged in 
`notified_events`, capturing relevant metadata.
2. **Creating Notifications:** Notifications are linked to events in 
`notifications`, enabling individual tracking for each recipient.
3. **Read Status:** Users mark notifications as read by updating `read_at`, 
enabling quick querying for unread notifications and visual badges.

Hope you find this helpful @gravitystorm 


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/1387#issuecomment-2443324208
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-10-30 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

5670d46dfbe671499bfe593b555939bb7e3aaf43  Add status filter to user's note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297/files/0b7d8b8711567f8a952e8c7c805faf911aa981f6..5670d46dfbe671499bfe593b555939bb7e3aaf43
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 status filter to user's note page (PR #5297)

2024-10-30 Thread Emin Kocan via rails-dev
### Description
This PR partly addresses #832 and serves as a smaller alternative to #5255 as 
suggested in comments. It aims to enhance user experience on user note pages by 
providing filtering functionality for notes by status. The `with_status` scope 
is added to the `Note` model, and the `NotesController` is updated accordingly. 
Default behavior remains unchanged since the status preselected value is set to 
`All` by default.

### How has this been tested?
Test for the new scope is added to note model test to ensure functionality is 
working as expected.

### Screenshots
Screenshot 2024-10-30 at 22 07 
56;
Screenshot 2024-10-30 at 22 07 
46;

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

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

-- Commit Summary --

  * Add status filter to user's note page

-- File Changes --

M app/controllers/notes_controller.rb (3)
M app/models/note.rb (10)
M app/views/notes/index.html.erb (14)
M config/locales/en.yml (4)
M test/models/note_test.rb (21)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297
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 pop-up or other obvious notification for getting a message or changeset comment (#3182)

2024-10-28 Thread Emin Kocan via rails-dev
Did a bit research on some libraries that offer such functionalities and 
implementation details that we could potentially use for this:
### 1. [notifications-rails](https://github.com/jonhue/notifications-rails)
`notifications-rails` provides a modular structure for creating, rendering, 
pushing, and managing settings for notifications, with each module backed by 
the database. This setup supports on-site notifications with user-configurable 
preferences, allowing users to see unread counts and mark notifications as read.

**Database Structure and Requirements:**

- **New Tables:**
  - **`notifications`:** Main table storing each notification. Key columns:
- `target_id` and `target_type` (polymorphic association for the recipient)
- `object_id` and `object_type` (polymorphic link to the notification’s 
subject, e.g., a `Comment`)
- `metadata` (JSON for additional data), `category` (notification type), 
and `read` (boolean for read/unread).
  - **`notification_groups` (optional):** Allows bulk notifications for groups 
(e.g., subscribers).

- **Modified Tables:**
  - **User Table**: Adds optional columns for `read_notification_count` and 
`unread_notification_count` to cache unread counts, and `settings` for storing 
user-specific notification preferences.

- **Indexes:**  
  - Indexes on `target_id`, `target_type`, and `category` for quick lookups in 
the `notifications` table.

**Database Workflow:**
1. **Creating Notifications:** Notifications are inserted into `notifications` 
with details on target, subject, metadata, and category.
2. **Marking as Read:** Updating `read` marks notifications as read, and 
`unread_notification_count` can be adjusted for caching.
3. **Bulk Notifications:** The `notification_groups` table can manage batch 
notifications by defining recipient groups.

### 2. [noticed](https://github.com/excid3/noticed)

`noticed` offers individual and bulk notifications, supporting multiple 
delivery methods, such as ActionCable for real-time updates. It’s suitable for 
creating on-site notifications tied to specific events, each managed through 
ActiveJob for asynchronous processing.

**Database Structure and Requirements:**

- **New Tables:**
  - **`notified_events`:** Logs each event that triggers notifications. Key 
columns:
- `type` (event type, e.g., “NewCommentNotifier”)
- `record_id` and `record_type` (polymorphic association to link with the 
event’s subject, e.g., `Post`)
- `params` (JSON to store event details, like message and URL).
  
  - **`notifications`:** Links notifications to events and tracks each 
recipient. Key columns:
- `event_id` (foreign key to `notified_events`), `recipient_id` and 
`recipient_type` (polymorphic association for the recipient), and `read_at` 
(datetime for read status).

- **Modified Tables:**
  - **User Table**: No required modifications, but adding `has_many 
:notifications` makes it easier to manage notifications.

- **Indexes:**
  - Indexes on `type`, `record_type`, and `record_id` in `notified_events` for 
efficient filtering and quick retrieval.

**Database Workflow:**
1. **Logging Events:** Each new event (e.g., a comment) is logged in 
`notified_events`, capturing relevant metadata.
2. **Creating Notifications:** Notifications are linked to events in 
`notifications`, enabling individual tracking for each recipient.
3. **Read Status:** Users mark notifications as read by updating `read_at`, 
enabling quick querying for unread notifications and visual badges.

Hope you find this helpful @gravitystorm 




-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/3182#issuecomment-2441918823
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 filtering functionality to user notes page (PR #5255)

2024-10-25 Thread Emin Kocan via rails-dev
As for splitting this into multiple smaller PRs, I would prefer we merge this 
as a complete feature/functionality. Adding filters one by one would 
significantly increase the time required and might leave the feature feeling 
incomplete. This PR covers all the intended functionality cohesively, and it 
seems better to refine and adjust post-merge if needed rather than delay 
further by segmenting the changes.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2435237921
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 filtering functionality to user notes page (PR #5255)

2024-10-31 Thread Emin Kocan via rails-dev
Closed #5255.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#event-15027313160
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] Contribution activity on user page (Issue #5298)

2024-10-31 Thread Emin Kocan via rails-dev
### Problem

Currently, the OpenStreetMap user profile page lacks a comprehensive 
contribution activity feed similar to those seen on platforms like GitHub. This 
means users cannot easily track or showcase their activity across changeset 
edits, note interactions, diary entries, and GPS trace uploads. This gap limits 
the visibility of a user's contributions and engagement within the community.

### Description

This is proposal for adding a user activity feed to the OpenStreetMap profile 
page to provide a detailed view of contributions. This would enhance user 
interaction by showcasing a summary of recent actions in a structured and 
engaging format.

**Technical Proposal**
The implementation will be split into multiple Pull Requests (PRs) for 
incremental development:

**PR-1-1: Database Schema Setup**
- **Add `user_activities` table**:
  - Define columns: `id`, `user_id` (indexed), `activity_type`, `object_type`, 
`object_id`, `metadata`, `created_at` (indexed).
- **Tests**:
  - Write tests to ensure the schema setup functions correctly.
- **Data Retention Policy**:
  - Establish an archival policy for data older than a specified period (e.g., 
one year).

**PR-1-2: Model Layer - UserActivity Model and Basic Tracking Logic**
- **Create `UserActivity` Model**:
  - Define core associations (`belongs_to :user`) and basic validations.
- **Model Callbacks for Logging**:
  - Implement `after_create` and `after_update` callbacks in relevant models 
(`Changeset`, `Note`, `DiaryEntry`, `GPSTrace`) for logging actions.
- **Shared Logic**:
  - Create an `ActivityTrackable` concern for models needing activity tracking 
logic.
- **Tests**:
  - Ensure the concern and model validations are covered by comprehensive tests.

**PR-2: Controller Layer - User Activity Controller**
- **Create `UserActivityController`**:
  - Implement an `index` action to fetch user activities with support for 
pagination, filtering, and sorting.
- **Querying**:
  - Utilize scopes in the `UserActivity` model for efficient filtering and 
preloading of data.
- **Tests**:
  - Write controller tests to validate that scoped queries return correct 
results.

**PR-3: Frontend Layer - Activity Feed Display**
- **Implement Activity Feed Partial**:
  - Create a partial to display user activities in the profile section with a 
standardized format.
- **Integration Tests**:
  - Conduct view tests to confirm the feed works as expected.
- **Pagination**:
  - Implement infinite scroll or use cursor-based pagination to maintain 
consistency across the section.

### Screenshots

https://github.com/user-attachments/assets/14e22309-3410-460d-90d4-9e56ce59278a";>

This is the fast mockup i came up with. Could be modified to fit our use case a 
bit better according to feedback:

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5298
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 filtering functionality to user notes page (PR #5255)

2024-10-31 Thread Emin Kocan via rails-dev

I opened new PR #5297 according to suggestion here. Closing this as not planed 
for now.

> You can make a pull request that just adds the status filter. That doesn't 
> depend on pagination and shouldn't tank the performance because:
> 
> - status is checked anyway to see if the note is not hidden
> 
> - we're doing a similar thing in the api



-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2449349900
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-10-31 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

0d0045ea6753ca3e830d03ec81a8640f0fa942e6  Add status filter to user's note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297/files/5670d46dfbe671499bfe593b555939bb7e3aaf43..0d0045ea6753ca3e830d03ec81a8640f0fa942e6
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] Allow Filtering of Notes by Status (Open/Closed) on User Profile (PR #5239)

2024-09-25 Thread Emin Kocan via rails-dev
This PR implements a basic filtering functionality for the notes displayed on a 
user's profile page, addressing part of the long-standing request from 
issue #832.

 Key Changes:
- Introduced tabs for filtering notes by status (Open/Closed) on the user 
profile page.
- When no status is specified in the URL, the **Open Notes** tab is selected 
and displayed by default.
- The URL is updated with a `status` parameter to reflect the current filter 
state, ensuring clarity for users and enabling easy sharing/bookmarking of 
filtered views.
- Maintains existing pagination and ordering logic, ensuring consistency with 
the current note display behavior.

 Future Steps:
This is the first step toward more comprehensive sorting and filtering 
functionalities, such as sorting by creation date, solved date, and more, as 
requested in #832. By implementing this, we improve the visibility of open 
(unresolved) notes while preserving the ability to view both open and closed 
notes for users accustomed to that behavior.

 Screenshots:
Screenshot 2024-09-26 at 04 46 
21;

Screenshot 2024-09-26 at 04 46 
39;

Screenshot 2024-09-26 at 04 46 55;

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

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

-- Commit Summary --

  * Add filtering for notes by status on user profile

-- File Changes --

M app/controllers/notes_controller.rb (4)
M app/views/notes/index.html.erb (9)
M config/locales/en.yml (2)
M db/structure.sql (7)

-- Patch Links --

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

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5239
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] Allow filtering of notes by status (open/closed) on user profile (PR #5239)

2024-10-03 Thread Emin Kocan via rails-dev
Closed #5239.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5239#event-14500664143
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] Allow filtering of notes by status (open/closed) on user profile (PR #5239)

2024-10-03 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

971f63d1f613d8dccc00707ee5a25522a6447e1c  Add filtering by status and date to 
user note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5239/files/ef9ad5484e917681c7bf36b9ef8084ca312c0975..971f63d1f613d8dccc00707ee5a25522a6447e1c
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] Allow filtering of notes by status (open/closed) on user profile (PR #5239)

2024-10-03 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

5047b5396739ee418ad6499b25c17a967b0b1a8e  Add filtering by status and date to 
user note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5239/files/14ac88714af3e8220c01c2c46a6138457fb23fa8..5047b5396739ee418ad6499b25c17a967b0b1a8e
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-06 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

b56dc10cec6239c067cca43f38176627f51887a5  Add status filter to user's note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297/files/337d37e71d1070ffe729ab6b7c6d877dc7cce203..b56dc10cec6239c067cca43f38176627f51887a5
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-06 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -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
+  }

Fixed and updated the PR. Since this is the only place using the scope, I 
removed it to simplify the code and maintain consistency. If a need arises, we 
can reintroduce it later.

Also, I’ve added a test in the controller to cover the basic status filtering 
functionality. Let me know if further tests are needed.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297#discussion_r1831242593
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-06 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

5b87c908f21977ec6908858b4f60b9a80d51e14d  Add status filter to user's note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297/files/0d0045ea6753ca3e830d03ec81a8640f0fa942e6..5b87c908f21977ec6908858b4f60b9a80d51e14d
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] Dark Mode (#2332)

2024-11-07 Thread Emin Kocan via rails-dev

Great work, thank you. I will take a look at PRs mentioned, in the meantime if 
you need help with any of these I'm happy to jump in. 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/2332#issuecomment-2461750909
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] Dark Mode (#2332)

2024-11-07 Thread Emin Kocan via rails-dev

I've been exploring these updates, and with Bootstrap 5.3 now in use, I 
believe we can implement them with minimal code adjustments.

> - **Refactor the background color for `content-heading` to use theme 
colors instead of a standard color variable**

This seems to be working fine locally, so I assume it’s already been addressed.

> - **Refactor our table striping override**

Tables could benefit from some additional styling adjustments to better align 
with the overall UI appearance in dark mode. I’d be happy to dive deeper into 
this and prepare a PR. Here’s how the notes table currently appears:
Screenshot 2024-11-07 at 09 22 48;

I also reviewed other pages across the site, and so far, everything else seems 
consistent.

I tried applying the suggested filter styling to the map, but I couldn't 
locate the correct class in the code. For reference, here’s the suggested style:

> `invert(100%) hue-rotate(180deg) brightness(95%) contrast(90%)`

@krjan02 @pkrasicki Would you know which class should be targeted to apply this 
effect to the map? 

If we’re aligned on this approach, I'm keen to open a PR for this. 
Additionally, would you be interested in styling the map with this filter as 
mentioned in the comments or perhaps adding a dropdown with options like dark 
mode (UI), dark mode (UI + map), and default mode?
@tomhughes @AntonKhorev @gravitystorm 

 Additional Screenshots
Screenshot 2024-11-07 at 09 36 
59;
Screenshot 2024-11-07 at 09 40 
20;


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/2332#issuecomment-2461656428
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] Guidance on Enhancing OSM Profile Pages with Contribution Data (Issue #5356)

2024-11-25 Thread Emin Kocan via rails-dev
Thank you for the clarification! Yes, we're focusing on the 
openstreetmap-website and aim to integrate this directly. Testing queries and 
performance locally with large datasets would ensure reliability at scale. For 
development, we can always mock or use a small subset of data to simplify 
testing.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5356#issuecomment-2498890643
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-18 Thread Emin Kocan via rails-dev
@kcne pushed 1 commit.

738e66afa6231390f52679fe8b343478643f5684  Add status filter to user's note page

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297/files/b56dc10cec6239c067cca43f38176627f51887a5..738e66afa6231390f52679fe8b343478643f5684
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-18 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -6,6 +6,20 @@
:commented => tag.span(t(".subheading_commented"), :class => "px-2 
py-1 bg-body") %>
 <% end %>
 
+<%= form_with :url => url_for(:controller => "notes", :action => "index"), 
:method => :get, :data => { :turbo => true } do %>
+  
+
+  <%= label_tag :status, t(".status") %>
+  <%= select_tag :status,
+ options_for_select([[t(".all"), "all"], [t(".open"), 
"open"], [t(".closed"), "closed"]], params[:status] || "all"),
+ :class => "form-select" %>
+
+
+  <%= submit_tag t(".apply"), :class => "btn btn-primary" %>

Couldn't realize why this was happening, I didn't like it either. Fixed now. 
Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297#discussion_r1846676919
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-18 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -6,6 +6,20 @@
:commented => tag.span(t(".subheading_commented"), :class => "px-2 
py-1 bg-body") %>
 <% end %>
 
+<%= form_with :url => url_for(:controller => "notes", :action => "index"), 
:method => :get, :data => { :turbo => true } do %>

Missed this one. Refactored. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5297#discussion_r1846675030
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] Guidance on Enhancing OSM Profile Pages with Contribution Data (Issue #5356)

2024-11-25 Thread Emin Kocan via rails-dev
### Problem

We want to enhance the OSM profile pages to include more engaging and 
informative features, similar to [HDYC](https://hdyc.neis-one.org/). The goal 
is to display user contribution summaries, activity patterns, and insights into 
changesets. However, we're unsure about the best approach to efficiently query 
and integrate this data, especially within the constraints of the existing OSM 
infrastructure.

Key challenges include:
1. **Data Import**: 
   - Changesets in the planet OSM files contain user data, but tools like 
Osmosis and Osmium cannot directly link users to contributions in a database.
   - Full history dumps provide the necessary data but are massive (~223 GB 
compressed, ~3.7 TB uncompressed) and complex to process.
2. **Performance**: 
   - We’re uncertain how querying user contribution data at scale would impact 
performance.
3. **Integration**:
   - Ideally, we want to avoid additional overhead or external systems, 
preferring a solution within the existing OSM infrastructure.


### Description


To achieve this, we’re considering several approaches, but we need guidance on:
- Whether anyone has implemented similar features before and has insights to 
share.
- The feasibility of querying contribution data directly versus preprocessing 
it into a database.
- How to efficiently test and benchmark the performance of querying user 
contribution data.
- Whether a solution can be integrated into the existing OSM infrastructure or 
if a separate system is necessary.

 Questions

1. Has anyone worked on similar enhancements or features for OSM profile pages? 
If so, what approach did you take?
2. Are there recommended workflows or tools (e.g., Osmium, 
osm-history-renderer) for importing and querying user contribution data 
efficiently?
3. How can we test and compare the performance of different approaches without 
committing to a full-scale import?
4. Is it possible to leverage existing infrastructure to handle these queries, 
or would setting up a separate processing system be unavoidable?

We welcome any advice, insights, or references to similar projects to help us 
move forward. @gravitystorm @tomhughes @AntonKhorev @pa5cal

### Screenshots

_No response_

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5356
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] Adds note tags support (PR #5344)

2024-12-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -14,6 +14,10 @@ xml.note("lon" => note.lon, "lat" => note.lat) do
 
   xml.date_closed note.closed_at if note.closed?
 
+  note.tags.each do |k, v|

This code is duplicated multiple times, consider creating a shared helper 
method to reuse instead.

> +  validates :note, :associated => true
+  validates :k, :v, :allow_blank => true, :length => { :maximum => 255 }, 
:characters => true
+  validates :k, :uniqueness => { :scope => :note_id }

To ensure stronger data integrity, we can enforce that k and v are always 
present, within length limits, and unique per note. Additionally, using 
`presence: true` for note simplifies and improves performance compared to 
`associated: true`

```ruby
validates :note, presence: true
validates :k, presence: true, length: { maximum: 255 }, uniqueness: { scope: 
:note_id }
validates :v, presence: true, length: { maximum: 255 }
```

> +class CreateNoteTags < ActiveRecord::Migration[7.2]
+  def change
+# Create a table and primary key
+create_table :note_tags, :primary_key => [:note_id, :k] do |t|
+  t.column "note_id", :bigint, :null => false
+  t.column "k",  :string, :default => "", :null => false
+  t.column "v",  :string, :default => "", :null => false
+
+  t.foreign_key :notes, :column => :note_id, :name => "note_tags_id_fkey"
+end
+  end
+end

This migration works well for ensuring uniqueness with a composite primary key. 
However, if querying by tags (e.g., `{k: "created_by", v: "openstreetmap"}`) is 
expected, a surrogate primary key (`id`) with additional indexes might be more 
efficient. For example:

```ruby
create_table :note_tags do |t|
  t.bigint :note_id, null: false
  t.string :k, null: false, default: ""
  t.string :v, null: false, default: ""

  t.timestamps
end

add_index :note_tags, [:note_id, :k], unique: true   # Enforce uniqueness
add_index :note_tags, [:k, :v]  # Optimize tag-based 
queries
```

This approach aligns with Rails conventions, simplifies queries, and allows 
adding indexes for tag lookups (`k`/`v`).

> @@ -83,12 +85,30 @@ def create
   lat = OSM.parse_float(params[:lat], OSM::APIBadUserInput, "lat was not a 
number")
   comment = params[:text]
 
+  # Extract the tags parameter (if it exists)
+  tags = []
+  if params[:tags].present?
+# Split by commas to get individual key-value pairs
+pairs = params[:tags].split(",")
+
+# For each pair in parameters, store it in tags variable
+pairs.each do |pair|
+  key, value = pair.split(":", 2)
+  tags << { :k => sanitize(key), :v => sanitize(value) } if key && 
value
+end
+  end

I agree with Tom here. Also independent of implementation, validation here 
would be necessary in my opinion.

 Two alternative approaches that could be useful here is to:
- use nested form parameters:
```
tags[created_by]=OpenStreetMap-Website&tags[editor]=JOSM
```
- using a prefix for tag related properties:
```
tag_created_by=OpenStreetMap-Website&tag_editor=JOSM
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5344#pullrequestreview-2475434346
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] Calendar heatmap of user contributions on user profile (Issue #5373)

2024-12-03 Thread Emin Kocan via rails-dev
### Problem

The current user profile page provides a detailed summary of contributions but 
lacks a visual representation of activity trends. A calendar heatmap, similar 
to GitHub's user contribution calendar, would make it easier to understand user 
engagement over time at a glance.

### Description

I have implemented a working calendar heatmap locally for changesets using the 
[cal-heatmap-rails 
library](https://rubygems.org/gems/cal-heatmap-rails/versions/3.6.2?locale=en) 
for display, querying changesets directly from the database. The heatmap 
visually displays daily contributions, with color intensity representing the 
number of changes for the changset on the given date.

**Additional Ideas:**
- Not sure if we should count changsets only in initial version or add diary 
entries and note creations/comments as well.

- Since we have dark mode on OSM plan is for that to be reflected on the 
heatmap as well.

**Next Steps:**  
I’ll open a PR after refining the implementation. In the meantime, I’d love to 
hear your thoughts and suggestions on this feature and the proposed 
contribution types.


### Screenshots

https://github.com/user-attachments/assets/dcb68c4f-f82f-4faa-b87b-d42ee29e3389";>


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5373
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 user profile heatmap visualization for contributions (PR #5402)

2024-12-24 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

32ed629cd8d7af0e1d5615a352c44e7eece4057e  Add heatmap javascript logic, styles 
and localization
0a8fc1422aaa55c5b0c7e7220a99e4bda6bed9c5  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/b134a0ad97ecf208f7772de29efec73907b7a1ea..0a8fc1422aaa55c5b0c7e7220a99e4bda6bed9c5
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 user profile heatmap visualization for contributions (PR #5402)

2024-12-24 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

27333136065e1eb2f34b3b8d0275f2f03692b984  Add heatmap javascript logic, styles 
and localization
af02b6aecf6f39ac3694f00758f920c545cd19b1  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/0a8fc1422aaa55c5b0c7e7220a99e4bda6bed9c5..af02b6aecf6f39ac3694f00758f920c545cd19b1
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 user profile heatmap visualization for contributions (PR #5402)

2024-12-26 Thread Emin Kocan via rails-dev
I've updated the code to account for the "Preferred Website Color Scheme." 
Theme settings should now be working correctly.

**UX Thoughts:**  
- **Disabling Feature in Profile:** Data is already public and open, so I 
believe all mappers should have a contribution graph. This feature provides 
valuable insights for more experienced mappers as a reference point to 
understand others' activity.  
- **"Never Contributed" Case:** Showing an empty heatmap with a fallback 
message seems like a good approach. From a UX perspective, maintaining a 
consistent layout across profiles would be more intuitive and visually cohesive.

**Code Comments:**  
- **Localizing Starting Week:** I attempted to localize the starting week as 
suggested but couldn't find a clean solution due to limitations with the 
`ghDays` subdomain. If anyone has suggestions, I'm open to exploring further.  
- **Separate JS Files:** There's already a `heatmap.js` file containing 
heatmap-related functionality, included in `application.js` for precompilation. 
I'm hesitant to introduce additional files unless there's a clear benefit. 
Consolidating heatmap logic here seems to make sense for now.  
- **Updating `package.json`:** I’ll update the `package.json` as part of the 
changes, but I’m unsure if additional configuration is needed. Please let me 
know if there are any dependencies or settings to consider. @gravitystorm 

Let me know if further adjustments or discussions are needed!


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#issuecomment-2562356509
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +.heatmap-wrapper {
+  position: relative;
+  height: 130px;
+}
+
+.heatmap-labels {
+position: absolute;
+  top: 2px;
+  left: 17px;
+  padding-top: 21px;
+  z-index: 1;
+  overflow-y: hidden;
+}
+
+.heatmap {
+  margin-left: 50px;
+  height: 100%;
+  z-index: 0;
+}

I decided to remove `heatmap.css` completely since we opted to using npm 
package and linking stylesheet inside `show.html.erb` directly. Since it's only 
18 lines of code I think we can leave it in `common.scss` for now. If you still 
think I should move it to separate file instead, just let me know.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939893538
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  height: 100%;
+  z-index: 0;

I thought they did as precaution but they don't. Removed now and everything 
works ok. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939889069
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

fde2ab967b45609d6d4186f00500b51b39f8c55d  Add heatmap javascript logic, styles 
and dependencies
7680971278a6941ebf3ce639aa1dd4b4c07767cc  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/8a32593b005f706738fdfec7c8f775b5f01a5aa3..7680971278a6941ebf3ce639aa1dd4b4c07767cc
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



>  //= link_tree ../../../vendor/assets/iD/iD/img
 //= link_directory ../../../vendor/assets/iD/iD/data .json
 //= link_directory ../../../vendor/assets/iD/iD/locales .json
 //= link_directory ../../../vendor/assets/iD/iD/mapillary-js .css
 //= link_directory ../../../vendor/assets/iD/iD/mapillary-js .js
 //= link_directory ../../../vendor/assets/iD/iD/pannellum .js
 //= link_directory ../../../vendor/assets/iD/iD/pannellum .css
+//= link_directory ../../../vendor/assets/d3 .js
+//= link_directory ../../../vendor/assets/cal-heatmap .js
+//= link_directory ../../../vendor/assets/cal-heatmap .css
 

Alright, fixed now and rebased the commits accordingly. Thank you for putting 
effort into demonstrating how to properly do this!


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939879726
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  type: "threshold",
+  range: currentTheme === "dark" ? rangeColors : 
Array.from(rangeColors).reverse(),
+  domain: [10, 20, 30, 40]
+}
+  }
+}, [
+  [Tooltip, {
+text: (date, value) => getTooltipText(date, value, locale)
+  }]
+]);
+  }
+});
+
+function getThemeFromColorScheme(colorScheme) {
+  if (colorScheme === "auto") {
+return window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" 
: "light";

You were completely right. I tried removing the method, but instead of doing 
two things in one method or in if statement i just refactored the method and 
moved it around a bit. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939881436
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne pushed 4 commits.

b4abb7c36d77d8634bf116d6287ee698fa3e63ad  Add heatmap data caching and query 
for user contributions
754f79000ccce4a41eea7d2e09e1bc039b3feba7  Add heatmap container to user profile 
view
183a14f2827d398b0465153b336204ad205981bb  Add heatmap javascript logic, styles 
and dependencies
8a32593b005f706738fdfec7c8f775b5f01a5aa3  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/ad50c87e692aa87a3644027fb519c494b191e735..8a32593b005f706738fdfec7c8f775b5f01a5aa3
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  data: {
+source: heatmapData,
+type: "json",
+x: "date",
+y: "total_changes"
+  },
+  scale: {
+color: {
+  type: "threshold",
+  range: currentTheme === "dark" ? rangeColors : 
Array.from(rangeColors).reverse(),
+  domain: [10, 20, 30, 40]
+}
+  }
+}, [
+  [Tooltip, {
+text: (date, value) => getTooltipText(date, value, locale)

Fixed. Removed locale altogether since as you pointed out I can use I18n 
directly. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939882886
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  const translations = I18n.translations[locale] || I18n.translations.en;
+  const date = translations && translations.date;
+  const abbrMonthNames = date && date.abbr_month_names;
+
+  const months = abbrMonthNames || [];
+  return months[monthIndex + 1] || "";

This is great, makes code much cleaner and shorter. Removed method altogether 
and just used the code you provided in the text callback. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939884862
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  top: 2px;
+  left: 17px;
+  padding-top: 21px;

I had issues while making this work on UI with positioning. Fixed now, removed 
padding top and relied on top property completely instead. Everything works 
properly. Marking as resolved.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1939887171
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

4347c00fb4b694a918dc9134c2f8f53d22392525  Add heatmap javascript logic, styles 
and dependencies
ad50c87e692aa87a3644027fb519c494b191e735  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/661db86cbcb0d02f9a2451258bef5f4e81abc264..ad50c87e692aa87a3644027fb519c494b191e735
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-03 Thread Emin Kocan via rails-dev
@kcne pushed 4 commits.

5969abfcaf1c424db763488dcf05738884750192  Add heatmap data caching and query 
for user contributions
66834e32332adf773e8866f55452bd1379bdfa46  Add heatmap container to user profile 
view
49b1c4192368f34bf891d64555af5900ad7a79ac  Add heatmap javascript logic, styles 
and dependencies
661db86cbcb0d02f9a2451258bef5f4e81abc264  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/7ebadd0eafcf62c30241db10167726bbe9424eae..661db86cbcb0d02f9a2451258bef5f4e81abc264
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  
+
+  

Refactored the code to use flexbox instead. It is indeed much simpler, as I 
could get rid of some custom css classes. Thanks once again.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1942244049
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

9fc9ed65c63ad95897af6665fe9275ccb8976a4f  Add javascript logic, styles and 
heatmap in view
341fe1378fe6d3ebf260e1715329affb1aa1fa5f  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/ec5f4a5edc56db8a1a2bb14244cffdac85661466..341fe1378fe6d3ebf260e1715329affb1aa1fa5f
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +<% if @heatmap_data.present? %>
+  
+<%= tag.div(:id => "cal-heatmap", :data => { :heatmap => 
@heatmap_data.to_json }) %>
+  
+<% end %>

Okay, kept the if statement as it is now and also removed styling mentioned and 
`console.warn` statment from the javascript code. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1940797988
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

2c62802c3d056edb43f3e20e3e8a5d7d3c8fad05  Add heatmap container to user profile 
view
e1bc1b99783548e8382e29fd8d7fb5ea61a83fb6  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/7680971278a6941ebf3ce639aa1dd4b4c07767cc..e1bc1b99783548e8382e29fd8d7fb5ea61a83fb6
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +  <%= stylesheet_link_tag "heatmap" %>
+  <%= javascript_include_tag "heatmap" %>

Removed from this commit and added later when the files are added. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1940796304
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +<% if @heatmap_data.present? %>
+  
+<%= tag.div(:id => "cal-heatmap", :data => { :heatmap => 
@heatmap_data.to_json }) %>
+  
+<% end %>

When we decide the conditions for renedering we can refactor the if statement 
as necessary. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1940799285
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -237,11 +237,29 @@
 
 <%= @user.description.to_html 
%>
 
-<% if @heatmap_data.present? %>
-  
-<%= tag.div(:id => "cal-heatmap", :data => { :heatmap => 
@heatmap_data.to_json }) %>
+

Removed that div entirely, thank you. I thought  it would help make it 
responsive on small screens in bootstrap grid, but apparently it doesn't do 
anything in this case.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1940802623
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +# Test user with no changesets
+user_without_changesets = create(:user)
+get user_path(user_without_changesets)
+assert_response :success
+assert_select "div#cal-heatmap", 1

Updated the test to reflect latest changes. Thank you.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1942227578
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

3fc5a320930d24208aabeaf0acfb5af4802c8cdd  Add javascript logic, styles and 
heatmap in view
ec5f4a5edc56db8a1a2bb14244cffdac85661466  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/f504d36d34750e99d9fcf190d4f9cdc1d8bd25e8..ec5f4a5edc56db8a1a2bb14244cffdac85661466
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-04 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

fbe7865e0255c06d8e5760faa945c464009007c8  Add heatmap container to user profile 
view
ccaeee9113382d874dd015d9f5106f9296f38b39  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/e1bc1b99783548e8382e29fd8d7fb5ea61a83fb6..ccaeee9113382d874dd015d9f5106f9296f38b39
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-05 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

7a2fd38e7a2b86aaa1cfb13f91972b8480b1d2c9  Add javascript logic, styles and 
heatmap in view
a068ef7ad03658871fbd1723e20449126f76f32d  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/341fe1378fe6d3ebf260e1715329affb1aa1fa5f..a068ef7ad03658871fbd1723e20449126f76f32d
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-05 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> @@ -3300,6 +3300,10 @@ en:
   show_address: Show address
   query_features: Query features
   centre_map: Centre map here
+heatmap:
+  tooltip:
+contributions: "%{count} contributions on %{date}"

Updated keys and js code respectively. Now we got `no_contributions`, 
`contributions` with `%{count}` and `one_contribution`. 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1943625554
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-05 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

840decddc6af04be60ff5ef398f9fffd113b407e  Add heatmap data caching and query 
for user contributions
38f909333e62e46665f0e0bb123573b79023bc87  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/a068ef7ad03658871fbd1723e20449126f76f32d..38f909333e62e46665f0e0bb123573b79023bc87
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-10 Thread Emin Kocan via rails-dev
@kcne pushed 2 commits.

373ded893fce0b68a7767ca43974d92cf864d30e  Add heatmap data caching and query 
for user contributions
7ff09b4fa1fa531a36fbbdd65a4bf54ac5f83091  Add tests for heatmap data in 
UsersController

-- 
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402/files/1caa215e8274b32402358276bfd596b8a8f1c01b..7ff09b4fa1fa531a36fbbdd65a4bf54ac5f83091
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 user profile heatmap visualization for contributions (PR #5402)

2025-02-10 Thread Emin Kocan via rails-dev
@kcne commented on this pull request.



> +return value === 1 ?
+  I18n.t("javascripts.heatmap.tooltip.one_contribution", { date: 
localizedDate }) :
+  I18n.t("javascripts.heatmap.tooltip.contributions", { count: value, 
date: localizedDate });

Resolved. 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5402#discussion_r1949743003
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   >