[openstreetmap/openstreetmap-website] Add a basic when-last-changed feature to the history pages (PR #3535)

Tom Hughes notifications at github.com
Mon Apr 25 13:11:04 UTC 2022


@tomhughes requested changes on this pull request.



> @@ -142,8 +142,6 @@ $$;
 
 SET default_tablespace = '';
 
-SET default_table_access_method = heap;

Please remove this change, which is not related to this PR and is an artefact of your postgres version.

> @@ -31,4 +31,4 @@
 </div>
 <% end %>
 
-<%= render :partial => "tag_details", :object => common_details.tags %>
+<%= render :partial => "tag_details", :object => (local_assigns[:tag_history] or common_details.tags), :locals => { :is_history => local_assigns[:is_history] } %>

Is there a reason for using `local_assigns` in all these templates? A local value passed to `render` should be available as a normal variable I think, so just `is_history` would work as well as `local_assigns[:is_history]` here?

> @@ -75,4 +75,21 @@ def icon_tags(object)
   def name_locales(object)
     object.tags.keys.map { |k| Regexp.last_match(1) if k =~ /^name:(.*)$/ }.flatten
   end
+
+  def tags_with_version_info(current_tags, old_objects)
+    version_history = {}
+
+    current_tags.each do |t|
+      version_history[t.k] = { :value => nil, :version => -1 }
+    end
+    old_objects.each do |old|
+      old.old_tags.each do |t|
+        if version_history.include?(t.k) && version_history[t.k][:value] != t.v
+          version_history[t.k] = { :value => t.v, :version => old.version,
+                                   :changeset => old.changeset, :timestamp => old.timestamp }
+        end

I think if you this needs to reset the version in the history to -1 for any tags which aren't in this version, otherwise I think if a tag is deleted and then reinstated with the same value the last edit will appear to be the version when it was originally added rather than the version where it was reinstated?

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

Message ID: <openstreetmap/openstreetmap-website/pull/3535/review/951820237 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20220425/63d6c507/attachment.htm>


More information about the rails-dev mailing list