export/import group data via json file #4632

robguthrie · 2018-04-25T11:20:53Z

No description provided.

gdpelican · 2018-05-07T02:00:50Z

@robguthrie Does this need additional work to be merged? It looks like the existing code doesn't really do anything other than memberships...

robguthrie · 2018-05-07T02:10:25Z

Please leave it for now. I modified it for memberships to repair the damage last week. I'll pick it up again soon. It's not super fast but it works well enough to get jobs done.

…

On Mon, May 7, 2018 at 9:00 AM, James Kiesel ***@***.***> wrote: @robguthrie <https://github.com/robguthrie> Does this need additional work to be merged? It looks like the existing code doesn't really do anything other than memberships... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4632 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAdr30P_yORjY1eIHASn_PkUgp1tmQtGks5tv6rTgaJpZM4TjPIS> .

gdpelican · 2018-05-15T22:51:58Z

Would be nice to get this merged in, @robguthrie ; anything I can do to support?

robguthrie · 2018-05-30T18:03:04Z

Hi @gdpelican. This version does not gobble memory when exporting large groups.. we stream data straight into the file with one json record per line. Ruby process stays around 100 mb rather than growing to 8gb.

We may want to think about how to use the .import method, maybe by doing a group by table then import after dropping/ignoring existing records.

Anyway, I'm looking to complete this over the next day or so. Hello.

robguthrie · 2018-05-31T15:52:36Z

@gdpelican the export and import of our biggest groups can happen in 5 minutes or so each now, without crazy memory usage.

I'm thinking burndown lists now.

Consider who can export a group and it's subgroups. If a group admin cannot access a subgroup, do they get to export it?
Self service export - create background job and upload the file somewhere. I'd like to reuse our own attachments system for this somehow.
excluded fields. We're going to exclude some tokens and keys right? Which ones should we exclude?
attachment files. Need to consider what to do here. could be that it's handled on import rather than export. IE: download attachments from previous server on import and update their location.

robguthrie · 2018-06-02T19:37:17Z

Ok so we're now excluding user confidential fields.

So I think now we enable users to export json for any group they belong to.
They select the parent group then get told they'll be emailed when the file is available. They then get an email with a link to their json for the group. It will contain the top level group and all the subgroups they belong to in a single json file.

I'm thinking that we create an attachment owned by the user with no attachable, and add the exported json to it, then add a delete delayed job scheduled for a week later.

robguthrie · 2018-06-10T18:51:25Z

@gdpelican this is in final stages now. Just going to finish the email styling. CR appreciated

gdpelican · 2018-06-10T19:00:07Z

app/controllers/api/groups_controller.rb

@@ -37,6 +37,11 @@ def upload_photo
    respond_with_resource
  end

+  def export
+    service.export(group: load_and_authorize(:group), actor: current_user)


Pedantic, but you should be able to do

service.export(group: load_resource, actor: current_user)

because it should be the service's job to authorize whether you can export or not.

Kind of related. One thing that might be relevant is anonymous polls.
A data export would allow people to see who voted what.

If it were not for anonymous polls then I'd say that anyone who is a member should be able to export the group data.

If there are anonymous polls, I don't know if anyone should be able to download the data. It's a weird situation. What are your thoughts?

If a poll's anonymous, we should anonymize the export data.

Here's where I kinda wish we had serializers for these things, because it would mean we could make tweaks like this a bit more easily, rather than throwing scopes on the exportable_relations.

Instead of that though (I reckon it's a PITA), I think I'd prefer filtering out anonymous polls over to including them

# exportable_relations.rb has_many :exportable_polls, -> { where(anonymous: false) }, source: :polls

# group_export_service RELATIONS = [ ... 'exportable_polls' # (and not 'polls') ]

(NB that we don't use the 'discussion_polls' relation at the moment, because we're ensuring that a poll has the group_id set correctly if the discussion_id is set. A bit stateful-y, but it's worked so far.

gdpelican · 2018-06-10T19:04:18Z

app/models/formal_group.rb

@@ -48,7 +48,7 @@ class FormalGroup < Group
  belongs_to :default_group_cover

  has_many :subgroups,
-           -> { where(archived_at: nil).order(:name) },


I agree with this, but wonder if we need to check the /g/:key/subgroups query to make sure it's still returning the same thing?

gdpelican · 2018-06-10T19:05:34Z

app/models/guest_group.rb

+  end
+
+  def documents
+    Document.none


Hmmmm I wonder if we want to move the has_many :documents line from FormalGroup to Group instead of doing this.

gdpelican · 2018-06-10T19:06:43Z

app/models/membership.rb

@@ -89,6 +89,6 @@ def discussion_readers
  private

  def set_volume
-    self.volume = user.default_membership_volume if group.is_formal_group?
+    self.volume = user.default_membership_volume if id.nil? && group.is_formal_group?


Oh hmm what is this change needed for?

I think more idiomatic is self.new_record?

I've working to remove all unnecessary N+1 queries. Reading volume triggers a query in a couple of places (checking default membership volume and group volume) and it also pollutes the export because it returns a value that isn't actually what the column actually contains.

So that's why I've moved to using a method that clearly says it's giving a computed value for volume rather than overloading the simple accessor. I like it more and it hasn't really been a problem to change.

Oh, I was referring to the id.nil? addition.

gdpelican · 2018-06-10T19:07:54Z

app/serializers/discussion_reader_serializer.rb

@@ -33,10 +33,6 @@ def discussion_reader_id
    object.id
  end

-  def discussion_reader_volume


This gets used in discussion_model.coffee, I wonder if we need to account for that?

Also, wonder if volume changes are not strictly necessary for group export

A flag that this needs to be addressed in some way before merge; we can't be referencing discussionReaderVolume anywhere on the client if this method is taken out

gdpelican · 2018-06-10T19:12:01Z

app/jobs/group_export_job.rb

+  def perform(group, actor)
+    groups = actor.groups.where(id: group.all_groups)
+    filename = GroupExportService.export_filename_for(group)
+    GroupExportService.export(groups, filename)


I would have expected

filename = GroupExportService.export(group)

and then call group.all_groups from within the service

The plan was that anyone in a group can export the data. Meaning they should only be able to export data for groups they belong to. So I want to pass in only groups the user belongs to.

Ah, then better to pass in the actor here and do that line within the service. I think the filename method is out of place here, there's no reason to have two static methods on GroupExportService like this, since they're so intertwined.

filename = GroupExportService.export(group, actor) document = Document.create author: actor, file: File.open(filename, 'r'), title: filename UserMailer.group_export_ready(actor, group, document).deliver

I also wonder about creating an event for this so that we can easily track it and send it to other places in the future if we want (like, text me when it's done, or send me a push notification with a link to the thing, etc.)

GroupExported.publish!(group, actor, document)

This also maintains our current distance from our ideal of 'events are the only way to send emails within the app'

gdpelican · 2018-06-10T19:15:08Z

client/angular/components/group_page/group_actions_dropdown/group_actions_dropdown.coffee

+
+    $scope.openGroupExportModal = ->
+      ModalService.open 'ConfirmModal', confirm: ->
+        submit: -> Records.groups.export($scope.group.id)


We tend towards actions on the model, so

# group_model.coffee export: => @remote.post(@id, 'export')

submit: $scope.group.export

done, thanks

gdpelican · 2018-06-10T19:16:32Z

spec/controllers/api/groups_controller_spec.rb

@@ -12,6 +12,11 @@
    sign_in user
  end

+  describe 'export' do
+    it 'kicks off a group export job'


Would be good to write these, even if it's just a check for 200 and 403. Maybe check to ensure Document.count is incremented too.

gdpelican · 2018-06-10T19:51:22Z

app/services/group_export_service.rb

+  end
+
+  def self.import(filename)
+    tables = File.open(filename, 'r').map { |line| JSON.parse(line)['table'] }.uniq


There's some stuff that smells a bit here, but I haven't got a better solution off the top of my head given that (I'm assuming) we don't want to port all of the file into memory. I think json parsing each line of the file N+1 times is going to hurt us, and wonder if there's a way to avoid it.

I wouldn't be opposed to, say, iterating through each line of the file, parsing it, and then,
if the table matches the current table, klass.new and append it to an array of records
if the table doesn't match, klass.import, flush the existing array, and start a new one with the new table

Then we only iterate through the file once, run JSON.parse once per line, and still maintain support for weirdo input like a stray group in a middle of a big run of discussions or whatnot.

PS I believe import will silently fail imports with ids that exist already, which would be the same behaviour as this would exhibit

import totally fails if some of the ids already exist. so the check is necessary.

Thanks for helping to try and improve, but i think I'd like to call it as good enough for now. It's the best solution (fastest export for large groups by miles) after quite a few attempts and I'd like to move on.

gdpelican · 2018-06-10T19:56:01Z

app/services/group_export_service.rb

+  end
+
+  def self.export_filename_for(group)
+    "tmp/#{DateTime.now.strftime("%Y-%m-%d_%H-%M-%S")}_#{group.name.parameterize}.json"


A note that the group.name.parameterize will fail for guest groups, which we should be able to export just fine.

robguthrie · 2018-06-18T20:29:54Z

@gdpelican my show stopper issue with this is anonymous polls. What do you think we should do?

Some options that come to mind:

Do nothing and have a backdoor where users can see who said what
Mangle data on anonymous polls
Don't include anonymous polls in export

Any ideas?

…one week

…1 week from now

…t belong to group whose data is being requested

…after file.url has been updated with full path

robguthrie and others added 11 commits April 25, 2018 15:03

first pass at export group

b65a7b9

add migrate group service to export and import groups from json file

31fa3dd

add group export service to move data between loomio installs

78a3985

tidy up group exporter

972b946

ignore existing records

3d1b8e1

dont need that one

894e2ed

nor these really

82b2060

Add tests + better performance for MigrateGroupService

37ff9ac

Ensure json export

b865763

merge master

9f48008

hack GroupExportService to recover memberships

c77cf87

robguthrie added 3 commits May 24, 2018 17:52

Merge branch 'master' into copy-data

ccec44d

Merge branch 'master' into copy-data

89c16a0

do not baloon memory on export

ed13291

robguthrie added 4 commits May 31, 2018 13:36

need to defuck discussion_readers.volume method

e349ca9

dont overload table_name methods with db calling methods

2880358

dont need it

3cf0db0

fast enough export and import

6944102

remove user ip columns. exclude user confidental columns

c240a7f

robguthrie added 4 commits June 10, 2018 19:01

getting there

fc68d90

Merge branch 'master' into copy-data

9f7e1c2

self serve group data export

194c646

remove debug

6c3091d

gdpelican reviewed Jun 10, 2018

View reviewed changes

implement review feedback

8c4cb79

robguthrie and others added 18 commits August 4, 2018 10:49

Merge branch 'master' into copy-data

696393c

feat(email): fixed subject line for group export ready email

d7b16aa

chore(email): added link to email

a00c30e

feat(email): specify in email that link should only be available for …

c69a580

…one week

chore(style): added default layout to group_export_ready

4acbfad

chore(copy): case stuff

6ec1d00

feat(polls): only surface non-anonymous polls and related data

c2fd158

feat(polls): surface only exportable poll (i.e. non-anonymous) data

e3027c5

feat(jobs): enqueue a delayed job for document (attachment) deletion …

d89bdc6

…1 week from now

chore(tests): added test to make sure 403 is returned if user does no…

9785df1

…t belong to group whose data is being requested

chore(tests): added test to check that email is sent

132aad6

Merge branch 'master' into copy-data

e14a40f

fix(document): document url uses computed url

0f4bce3

fix(fs): specify /tmp folder for temp file

e26f879

fix(document): fixed condition

792c435

fix(document): reference the right attribute in view file

69443f0

fix(document): new, open, close before save

d028687

fix(document): using after_create callback to ensure url is set only …

9c710c9

…after file.url has been updated with full path

robguthrie merged commit dc27aef into master Oct 24, 2018

robguthrie deleted the copy-data branch October 24, 2018 01:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

export/import group data via json file #4632

export/import group data via json file #4632

robguthrie commented Apr 25, 2018

gdpelican commented May 7, 2018

robguthrie commented May 7, 2018 via email

gdpelican commented May 15, 2018

robguthrie commented May 30, 2018

robguthrie commented May 31, 2018 •

edited

robguthrie commented Jun 2, 2018

robguthrie commented Jun 10, 2018

gdpelican Jun 10, 2018

robguthrie Jun 12, 2018

gdpelican Jun 18, 2018

gdpelican Jun 18, 2018

gdpelican Jun 10, 2018

gdpelican Jun 10, 2018

robguthrie Jun 12, 2018

gdpelican Jun 10, 2018

robguthrie Jun 12, 2018

gdpelican Jun 18, 2018

gdpelican Jun 10, 2018

gdpelican Jun 10, 2018

gdpelican Jun 18, 2018

gdpelican Jun 10, 2018

robguthrie Jun 12, 2018

gdpelican Jun 18, 2018

gdpelican Jun 10, 2018

robguthrie Jun 12, 2018

gdpelican Jun 10, 2018

gdpelican Jun 10, 2018 •

edited

gdpelican Jun 10, 2018

robguthrie Jun 12, 2018

gdpelican Jun 10, 2018

robguthrie commented Jun 18, 2018

export/import group data via json file #4632

export/import group data via json file #4632

Conversation

robguthrie commented Apr 25, 2018

gdpelican commented May 7, 2018

robguthrie commented May 7, 2018 via email

gdpelican commented May 15, 2018

robguthrie commented May 30, 2018

robguthrie commented May 31, 2018 • edited

robguthrie commented Jun 2, 2018

robguthrie commented Jun 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdpelican Jun 10, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robguthrie commented Jun 18, 2018

robguthrie commented May 31, 2018 •

edited

gdpelican Jun 10, 2018 •

edited