[openstreetmap/openstreetmap-website] Refactor trace api_update to avoid duplicating tags (#1614)

[Andy Allan]

This PR refactors the trace api_update method to work on existing trace objects, rather than creating temporary 'shadow' trace objects. In doing so it refactors the `from_xml` and `from_xml_node` model methods to be instance methods instead of class methods.

Advantages:
* No longer duplicate tags (i.e. fixes #1600)
* makes updating a trace via xml more similar to updating via forms (which don't use shadow objects)

Concerns:
* Doesn't avoid deleting and recreating tag objects when the tags are the same (but neither does the `tagstring=` method, afaict)
* Other classes (Node, Changeset etc) all use the class method approach for `from_xml`, rather than instance methods. I don't know whether they should be refactored too, or whether there's some advantage of the class method approach that I haven't understood.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Add failing test demonstrating tag duplication when updating traces via the API.
  * Refactor the from_xml methods to act on existing trace objects.

-- File Changes --

    M app/controllers/trace_controller.rb (10)
    M app/models/trace.rb (26)
    M test/controllers/trace_controller_test.rb (17)

-- Patch Links --

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

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/1614
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20170818/a32f84f7/attachment.html>