[openstreetmap/openstreetmap-website] Order of comments in changeset comments feeds (PR #4944)

[Tom Hughes]

@tomhughes requested changes on this pull request.



> @@ -46,6 +46,14 @@ def test_feed
         assert_select "item", :count => 3
       end
     end
+
+    # Gets comment Ids from HTML and checks that they are in descending order
+    last_comment_id = -1
+    @response.body.split("Changeset comment ")[1..].each_with_index do |data, index|
+      id = Integer data.split("<")[0]

The normal way to convert a string to an integer is to use `.to_i` rather than calling the constructor like this.

> @@ -46,6 +46,14 @@ def test_feed
         assert_select "item", :count => 3
       end
     end
+
+    # Gets comment Ids from HTML and checks that they are in descending order
+    last_comment_id = -1
+    @response.body.split("Changeset comment ")[1..].each_with_index do |data, index|

It would be better to test against the parsed response rather than trying to use string operations to inspect HTML embedded in feed XML from the raw response.

What I'm not sure about is how easy that is when you're looking at the HTML content.

I think we should certainly be able to use he parsed response to find the feed entries though even if we then extract the ID from the HTML using string operations.

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

Message ID: <openstreetmap/openstreetmap-website/pull/4944/review/2159330885 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240704/3705382f/attachment-0001.htm>