<p><b>@tomhughes</b> requested changes on this pull request.</p>

<p>There's some kind of deep issues here about how hard we should try to keep our crazy schema consistent between the current and history tables that will only get worse when we do the object models...</p><hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426">test/controllers/old_node_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -90,6 +90,9 @@ def test_version
     xml_node = xml_doc.find("//osm/node").first
     nodeid = current_nodes(:node_with_versions).id
 
+    # Ensure that the current tags are propogated to the history too
+    propogate_tags(current_nodes(:node_with_versions), nodes(:node_with_versions_v4))
</pre>
<p>You can't spell propagate ;-)</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426">test/controllers/old_node_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -90,6 +90,9 @@ def test_version
     xml_node = xml_doc.find("//osm/node").first
     nodeid = current_nodes(:node_with_versions).id
 
+    # Ensure that the current tags are propogated to the history too
+    propogate_tags(current_nodes(:node_with_versions), nodes(:node_with_versions_v4))
</pre>
<p>Also I don't understand why this is needed - we haven't created any tags on the current version of the node so what is there to propagate?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426">test/controllers/old_node_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -161,6 +164,12 @@ def check_not_found_id_version(id, version)
   # Test that getting the current version is identical to picking
   # that version with the version URI call.
   def test_current_version
+    propogate_tags(current_nodes(:visible_node), nodes(:visible_node))
+    propogate_tags(current_nodes(:used_node_1), nodes(:used_node_1))
+    propogate_tags(current_nodes(:used_node_2), nodes(:used_node_2))
+    propogate_tags(current_nodes(:node_used_by_relationship), nodes(:node_used_by_relationship))
+    propogate_tags(current_nodes(:node_with_versions), nodes(:node_with_versions_v4))
+
</pre>
<p>Again I am obviously not understanding as I don't see that we have created anything yet that needs to be propagated?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426">test/controllers/relation_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -337,6 +337,7 @@ def test_create
   def test_update_relation_tags
     basic_authorization "test@example.com", "test"
     rel_id = current_relations(:multi_tag_relation).id
+    create_list(:relation_tag, 4, :relation => current_relations(:multi_tag_relation))
</pre>
<p>Now arguably here we should propagate as technically by creating tags on the current object that are not on the corresponding history object we have put the database into an invalid state. It probably doesn't actually affect anything though, but then I'm not sure we even need to create these tags? We'll still test whether adding the new one works...</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLQgG7il_GgNH_HXtJaqNSKw3qXMDks5q8jhQgaJpZM4KtvV_">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABWnLaGkYG-u3xstNIj38Ia6Xi7BBS7Iks5q8jhQgaJpZM4KtvV_.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@tomhughes requested changes on #1367"}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1367#pullrequestreview-7892426"}}}</script>