[openstreetmap/openstreetmap-website] add bulk_upload (#1995)

[Andy Allan]

I've had a look through this today. It's a large and complex PR touching one of the most complex parts of the codebase, so it's hard to review. But I want to say that I completely support the idea of refactoring the changeset upload processing to import entities in bulk, in order to improve performance.

I've merged with the current master and fixed the conflicts, since there's been some controller refactoring since this PR was made. You can see the results at https://github.com/gravitystorm/openstreetmap-website/tree/changeset_bulk . I haven't updated this PR, since the tests no longer pass.

It turns out that there is a problem somewhere between this PR and the updated version of [activerecord-import](https://github.com/zdennis/activerecord-import) that we are now using. This PR uses `v0.25.0` and we now use `v1.0.1`. I think it's caused by our use of [composite-primary-keys](https://github.com/composite-primary-keys/composite_primary_keys) which allows belongs_to relations to have a foreign key specified as an array instead of a string, e.g. 

```
belongs_to :old_node, :foreign_key => [:node_id, :version]
```
activerecord-import then blows up [when trying to convert this array to a symbol](https://github.com/zdennis/activerecord-import/blob/38cc26b21969a7c76a3e5b88cb79d6591ee09035/lib/activerecord-import/import.rb#L51). This [was introduced](https://github.com/zdennis/activerecord-import/commit/3e7cd50eac766c09fb8b1ff5f372459248ae2bd1) in activerecord-import `0.28.1`.

So I think there are two things that we should work on here now:

1) Extract the additional changeset upload tests into their own PR. They contain useful checks that @mmd-osm and @jiaxuyang worked on as part of this PR, and will come in useful at some point even if this PR isn't merged.

2) Figure out the best way forward with this foreign key problem. This could involve a combination of 
    * rewriting this PR
    * changes to activerecord-import to restore compatibility with composite-primary-keys
    * rewriting this PR to work with the [upcoming rails 6 "insert_all"](https://github.com/rails/rails/commit/91ed21b304c468db8ce9fd830312c151432935d0)
    * something else entirely

-- 
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/1995#issuecomment-479501602
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20190403/b1b4eb41/attachment.html>