Add bulk complete UI #2550

christinach · 2019-02-07T21:48:16Z

Bulk edit button displays on search results page when a collection facet
is selected.
search params are passed around so we can retrieve the same results
set
Bulk edit controller queues 1 update job per 50 resources

Closes #1860
Closes #2494
Closes #697
Other functionality moved to #2586

christinach · 2019-02-07T21:55:09Z

closes #697

hackartisan · 2019-02-12T20:59:51Z

hackartisan · 2019-02-12T21:00:39Z

escowles · 2019-02-12T21:10:18Z

app/controllers/bulk_edit_controller.rb

+  end
+
+  def resources_update
+    (_, document_list) = search_results(q: params["search_params"]["q"], f: params["search_params"]["f"])


Does this retrieve the whole set of results, or just the first page?

good question; we will look into this.

I'd recommend adding a FL key with "id" too if that works through BL.

can't seem to provide fl to the search builder.

escowles · 2019-02-12T21:12:45Z

logfile

@@ -0,0 +1,2 @@
+2019-02-11 10:48:40.115 EST [76779] FATAL:  lock file "postmaster.pid" already exists
+2019-02-11 10:48:40.115 EST [76779] HINT:  Is another postmaster (PID 501) running in data directory "/usr/local/var/postgres"?


Looks like a stray logfile got added by mistake.

probably that was mine

christinach · 2019-02-13T22:29:53Z

- Bulk edit button displays on search results page when a collection facet is selected. - search params are passed around so we can retrieve the same results set - Bulk edit controller queues 1 update job per 50 resources refs #697 still to do to close that ticket: - Add rights statement, rights note - Add search summary box to top of edit form Co-authored-by: christinach <actspatial@gmail.com>

Co-authored-by: Christina Chortaria <actspatial@gmail.com>

Co-authored-by: Anna Headley <anna.headley@gmail.com>

Wrap form in a panel Co-authored-by: Christina Chortaria <actspatial@gmail.com>

escowles · 2019-02-15T20:55:26Z

In manually testing this, I'm seeing some (but not all) of my objects get completed. For example, I bulk completed 76 objects in staging, and 22 were left pending. I created 150 pending objects in my local dev instance, and 65 were completed, and 85 were left pending.

I can try this again, but I think it might make sense to take a look at the search logic to see if objects being completed are changing the search results as we page through them.

hackartisan · 2019-02-18T17:09:40Z

@escowles i would totally believe that is happening. because it queues the batch update job and then queries solr for the next page.

I think we can resolve this by creating all the jobs first, then queuing them. But I'm not sure how to write a regression test for a race condition like this.

escowles · 2019-02-18T17:40:53Z

@HackMasterA Yes, I agree it would be very hard to test — I'd be fine with manually testing to make sure any updates behave as expected.

hackartisan · 2019-02-18T19:54:36Z

@escowles I updated the way the jobs are queued. do you want to try it now?

escowles · 2019-02-18T19:57:13Z

@HackMasterA Yep, I'll try it out now

escowles · 2019-02-18T20:36:47Z

I'm still seeing only a portion of the records get updated. I've got my local environment setup to test this — do you want me to see if I can get it to complete them all?

hackartisan · 2019-02-18T20:51:37Z

@escowles sure, thanks!

hackartisan · 2019-02-18T21:17:31Z

@escowles are the resources more or less identical? or do they have different metadata, etc?

escowles · 2019-02-18T21:18:50Z

I'm testing with identical resources (other than titles being "Resource #{n}". I think I've figured out why it's updating only the first page of resources (and have a fix).

hackartisan · 2019-02-18T21:37:58Z

i will update the test / fix rubocop probably in the morning.

app/controllers/bulk_edit_controller.rb

tpendragon · 2019-02-19T18:54:19Z

app/controllers/bulk_edit_controller.rb

+    args = {}.tap do |hash|
+      hash[:mark_complete] = (params["mark_complete"] == "1")
+    end
+    batches = prepare_batches


Can you just rename prepare_batches to batches and memoize?

tpendragon · 2019-02-19T18:56:49Z

app/jobs/bulk_update_job.rb

+# frozen_string_literal: true
+class BulkUpdateJob < ApplicationJob
+  def perform(ids:, args:)
+    return unless args[:mark_complete]


Why would this ever get queued without mark_complete? I mean, I guess it would right now, but wouldn't it be more efficient to just not queue a thing if it didn't need updated?

I think this was a sort of breadcrumb for the fact that we will add new options to the form as we go along. but it doesn't really make sense the way it is; I'll look at moving or removing it.

spec/features/bulk_edit_spec.rb

hackartisan · 2019-02-19T21:23:54Z

@tpendragon I answered some of these questions and made some changes. memoizing batches increased its ABC so I had to extract another method. take another look? let me know what you think.

tpendragon · 2019-02-19T21:29:19Z

app/jobs/bulk_update_job.rb

+        attributes = {}.tap do |attrs|
+          attrs[:state] = "complete" if args[:mark_complete] && !resource.state.include?("complete")
+        end
+        change_set.validate(attributes)


Should we check the value of this so it doesn't update it if there's a validation error? Maybe even have it throw an exception so this batch dies?

@tpendragon I just added this please take a look. Thanks!

tpendragon · 2019-02-21T18:44:41Z

app/jobs/bulk_update_job.rb

+          attrs[:state] = "complete" if args[:mark_complete] && !resource.state.include?("complete")
+        end
+        change_set.validate(attributes)
+        if change_set.changed?


I don't think this is going to do what you want. Should be if change_set.validate(attributes) or if change_set.valid?.

change_set.changed? will just tell you if a property is different, which it really should be if it's being bulk updated.

I'm actually a little concerned the tests below pass in light of this.

OH, I see what's going on. Ignore me.

christinach added Review and removed Review labels Feb 7, 2019

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch 2 times, most recently from b26958a to 8930700 Compare February 12, 2019 18:23

christinach force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from 8930700 to 0bed38d Compare February 12, 2019 19:34

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from 0bed38d to 3d5637c Compare February 12, 2019 20:56

hackartisan changed the title ~~[WIP] display collection bulk edit button when facet collection is selected~~ Add bulk complete UI Feb 12, 2019

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from 3d5637c to 0099255 Compare February 12, 2019 20:58

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from 0099255 to 0a7ad72 Compare February 12, 2019 21:01

escowles reviewed Feb 12, 2019

View reviewed changes

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch 2 times, most recently from 5adfe2b to 738469e Compare February 13, 2019 15:00

escowles previously approved these changes Feb 13, 2019

View reviewed changes

christinach dismissed escowles’s stale review via 31299bf February 13, 2019 18:13

christinach force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from c78c8c6 to 31299bf Compare February 13, 2019 18:13

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch 2 times, most recently from c22d211 to f352633 Compare February 13, 2019 19:41

christinach and others added 4 commits February 13, 2019 17:50

Use search builder directly to batch each page of solr results

9ff58e7

Co-authored-by: Christina Chortaria <actspatial@gmail.com>

Add a flash message after submiting the bulk edit form

02f7d4c

Co-authored-by: Anna Headley <anna.headley@gmail.com>

Add search constraints box to bulk edit form

9b3a5b6

Wrap form in a panel Co-authored-by: Christina Chortaria <actspatial@gmail.com>

christinach force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from 668b682 to 9b3a5b6 Compare February 13, 2019 22:51

christinach added Review and removed in progress labels Feb 13, 2019

escowles mentioned this pull request Feb 15, 2019

Expand bulk complete functionality #2586

Closed

3 tasks

tpendragon temporarily deployed to staging February 15, 2019 18:19 Inactive

hackartisan added 2 commits February 18, 2019 14:29

Separate results retrieval from job execution more discretely

0910b2f

Refactor to split search more cleanly from processing

3c5d6aa

hackartisan force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from 12f0e58 to 3c5d6aa Compare February 18, 2019 19:29

Explicitly setting the SearchBuilder's start parameter

00d11ce

escowles force-pushed the 697-bulk-edit-workflow-status-rights-statement branch from ed16036 to 00d11ce Compare February 18, 2019 22:00

Prepopulate the change set

bb231aa

tpendragon requested changes Feb 19, 2019

View reviewed changes

Refactors for review

14bf6ba

tpendragon reviewed Feb 19, 2019

View reviewed changes

Raise when change set is not valid

9042752

tpendragon requested changes Feb 21, 2019

View reviewed changes

tpendragon approved these changes Feb 21, 2019

View reviewed changes

tpendragon merged commit 2b8c4a8 into master Feb 21, 2019

tpendragon deleted the 697-bulk-edit-workflow-status-rights-statement branch February 21, 2019 18:47

tpendragon removed the Review label Feb 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bulk complete UI #2550

Add bulk complete UI #2550

christinach commented Feb 7, 2019 •

edited by hackartisan

Loading

christinach commented Feb 7, 2019

hackartisan commented Feb 12, 2019

hackartisan commented Feb 12, 2019

escowles Feb 12, 2019

hackartisan Feb 12, 2019

tpendragon Feb 12, 2019

hackartisan Feb 13, 2019

escowles Feb 12, 2019

christinach Feb 12, 2019

christinach commented Feb 13, 2019

escowles commented Feb 15, 2019

hackartisan commented Feb 18, 2019

escowles commented Feb 18, 2019

hackartisan commented Feb 18, 2019

escowles commented Feb 18, 2019

escowles commented Feb 18, 2019

hackartisan commented Feb 18, 2019

hackartisan commented Feb 18, 2019

escowles commented Feb 18, 2019

hackartisan commented Feb 18, 2019

tpendragon Feb 19, 2019

hackartisan Feb 19, 2019

tpendragon Feb 19, 2019

hackartisan Feb 19, 2019

hackartisan commented Feb 19, 2019

tpendragon Feb 19, 2019

hackartisan Feb 19, 2019

hackartisan Feb 20, 2019

tpendragon Feb 21, 2019

tpendragon Feb 21, 2019

		@@ -0,0 +1,2 @@
		2019-02-11 10:48:40.115 EST [76779] FATAL: lock file "postmaster.pid" already exists
		2019-02-11 10:48:40.115 EST [76779] HINT: Is another postmaster (PID 501) running in data directory "/usr/local/var/postgres"?

Add bulk complete UI #2550

Add bulk complete UI #2550

Conversation

christinach commented Feb 7, 2019 • edited by hackartisan Loading

christinach commented Feb 7, 2019

hackartisan commented Feb 12, 2019

hackartisan commented Feb 12, 2019

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

christinach commented Feb 13, 2019

escowles commented Feb 15, 2019

hackartisan commented Feb 18, 2019

escowles commented Feb 18, 2019

hackartisan commented Feb 18, 2019

escowles commented Feb 18, 2019

escowles commented Feb 18, 2019

hackartisan commented Feb 18, 2019

hackartisan commented Feb 18, 2019

escowles commented Feb 18, 2019

hackartisan commented Feb 18, 2019

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

hackartisan commented Feb 19, 2019

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

christinach commented Feb 7, 2019 •

edited by hackartisan

Loading