Skip to content

Commit

Permalink
Stop using asynchronous requests on the edit form
Browse files Browse the repository at this point in the history
These requests caused the lock to be lost, so the page couldn't be saved
(#3127). Completely removed the "Add parent relationships" section as
this was not locking the parent.  It was unclear how we should
simultaneously update multiple objects and what should happen if the
lock for one of them was stale.

This should also fix #3133 as we are no longer passing the
ordered_member_ids as an attribute to the work actor for update. Instead
we are using a custom attribute and only making adds and deletions
instead of rewriting the whole set.

Fixes #3133
Fixes #3127
  • Loading branch information
jcoyne committed Mar 14, 2017
1 parent a7ee5cb commit fca4aef
Show file tree
Hide file tree
Showing 34 changed files with 501 additions and 921 deletions.
70 changes: 70 additions & 0 deletions app/actors/sufia/actors/attach_members_actor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
module Sufia
module Actors
# Attach or remove child works to/from this work. This decodes parameters
# that follow the rails nested parameters conventions:
# e.g.
# 'work_members_attributes' => {
# '0' => { 'id' = '12312412'},
# '1' => { 'id' = '99981228', '_destroy' => 'true' }
# }
#
# The goal of this actor is to mutate the ordered_members with as few writes
# as possible, because changing ordered_members is slow. This class only
# writes changes, not the full ordered list.
#
# TODO: Perhaps this can subsume AttachFilesActor
class AttachMembersActor < CurationConcerns::Actors::AbstractActor
def update(attributes)
attributes_collection = attributes.delete(:work_members_attributes)
assign_nested_attributes_for_collection(attributes_collection) &&
next_actor.update(attributes)
end

private

# Attaches any unattached members. Deletes those that are marked _delete
# @param [Hash<Hash>] a collection of members
def assign_nested_attributes_for_collection(attributes_collection)
return true unless attributes_collection
attributes_collection = attributes_collection.sort_by { |i, _| i.to_i }.map { |_, attributes| attributes }
# checking for existing works to avoid rewriting/loading works that are
# already attached
existing_works = curation_concern.member_ids
attributes_collection.each do |attributes|
next if attributes['id'].blank?
if existing_works.include?(attributes['id'])
remove(attributes['id']) if has_destroy_flag?(attributes)
else
add(attributes['id'])
end
end
end

def ability
::Ability.new(user)
end

# Adds the item to the ordered members so that it displays in the items
# along side the FileSets on the show page
def add(id)
member = ActiveFedora::Base.find(id)
return unless ability.can?(:edit, member)
curation_concern.ordered_members << member
end

# Remove the object from the members set and the ordered members list
def remove(id)
member = ActiveFedora::Base.find(id)
curation_concern.ordered_members.delete(member)
curation_concern.members.delete(member)
end

# Determines if a hash contains a truthy _destroy key.
# rubocop:disable Style/PredicateName
def has_destroy_flag?(hash)
ActiveFedora::Type::Boolean.new.cast(hash['_destroy'])
end
# rubocop:enable Style/PredicateName
end
end
end
6 changes: 2 additions & 4 deletions app/assets/javascripts/sufia/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,8 @@ Sufia = {
},

relationshipsTable: function () {
var rel = require('sufia/relationships/table');
$('table.relationships-ajax-enabled').each(function () {
new rel.RelationshipsTable($(this));
});
var RelationshipsControl = require('sufia/relationships/control');
new RelationshipsControl($('[data-behavior="child-relationships"]'), 'work_members_attributes', 'tmpl-child-work');
},

selectWorkType: function () {
Expand Down
9 changes: 3 additions & 6 deletions app/assets/javascripts/sufia/autocomplete.es6
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ export class Autocomplete {
this.autocompleteLocation(selector);
break;
case "work":
var user = selector.data('user');
var id = selector.data('id');
this.autocompleteWork(selector, user, id);
this.autocompleteWork(selector);
break;
}
});
Expand Down Expand Up @@ -54,13 +52,12 @@ export class Autocomplete {
new lang.Language(field, field.data('autocomplete-url'))
}

autocompleteWork(field, user, id) {
autocompleteWork(field, excludeWorkId) {
var work = require('sufia/autocomplete/work')
new work.Work(
field,
field.data('autocomplete-url'),
user,
id
field.data('exclude-work')
)
}
}
3 changes: 3 additions & 0 deletions app/assets/javascripts/sufia/autocomplete/location.es6
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
export class Location {
constructor(element, url) {
this.url = url
console.log("Location const")

element.autocomplete(this.options());
}

options() {
return {
minLength: 2,
source: ( request, response ) => {
console.log("Requestin")
$.getJSON(this.url, {
q: request.term
}, response );
Expand Down
51 changes: 30 additions & 21 deletions app/assets/javascripts/sufia/autocomplete/work.es6
Original file line number Diff line number Diff line change
@@ -1,29 +1,38 @@
export class Work {
// Autocomplete for finding possible related works (child and parent).
constructor(element, url, user, id) {
// Autocomplete for finding possible related works.
constructor(element, url, excludeWorkId) {
this.url = url;
this.user = user;
this.work_id = id;
element.autocomplete(this.options());
this.excludeWorkId = excludeWorkId;
this.initUI(element)
}

options() {
return {
minLength: 2,
source: ( request, response ) => {
$.getJSON(this.url, {
q: request.term,
id: this.work_id,
user: this.user
}, response );
initUI(element) {
element.select2( {
minimumInputLength: 2,
initSelection : (row, callback) => {
var data = {id: row.val(), text: row.val()};
callback(data);
},
focus: function() {
// prevent value inserted on focus
return false;
ajax: { // instead of writing the function to execute the request we use Select2's convenient helper
url: this.url,
dataType: 'json',
data: (term, page) => {
return {
q: term, // search term
id: this.excludeWorkId // Exclude this work
};
},
results: this.processResults
},
complete: function(event) {
$('.ui-autocomplete-loading').removeClass("ui-autocomplete-loading");
}
};
}).select2('data', null);
}

// parse the results into the format expected by Select2.
// since we are using custom formatting functions we do not need to alter remote JSON data
processResults(data, page) {
let results = data.map((obj) => {
return { id: obj.id, text: obj.label[0] };
})
return { results: results };
}
}
6 changes: 4 additions & 2 deletions app/assets/javascripts/sufia/relationships.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
//= require sufia/relationships/table
//= require sufia/relationships/table_row
//= require sufia/relationships/control
//= require sufia/relationships/registry
//= require sufia/relationships/registry_entry
//= require sufia/relationships/work
83 changes: 83 additions & 0 deletions app/assets/javascripts/sufia/relationships/control.es6
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import Registry from './registry'
import Work from './work'

export default class RelationshipsControl {

/**
* Initializes the class in the context of an individual table element
* @param {jQuery} element the table element that this class represents
* @param {String} property the property to submit
* @param {String} templateId the template identifier for new rows
*/
constructor(element, property, templateId) {
this.element = element
this.registry = new Registry(this.element, this.element.data('paramKey'), property, templateId)

this.input = this.element.find("[data-autocomplete='work']")
this.warning = this.element.find(".message.has-warning")
this.addButton = this.element.find("[data-behavior='add-relationship']")
this.errors = null
this.bindAddButton();
}

validate() {
if (this.input.val() === "") {
this.errors = ['ID cannot be empty.']
}
}

isValid() {
this.validate()
return this.errors === null
}

/**
* Handle click events by the "Add" button in the table, setting a warning
* message if the input is empty or calling the server to handle the request
*/
bindAddButton() {
this.addButton.on("click", () => this.attemptToAddRow())
}

attemptToAddRow() {
// Display an error when the input field is empty, or if the work ID is already related,
// otherwise clone the row and set appropriate styles
if (this.isValid()) {
this.addRow()
} else {
this.setWarningMessage(this.errors.join(', '))
}
}

addRow() {
this.hideWarningMessage()
let data = this.searchData()
this.registry.addWork(new Work(data.id, data.text))

// finally, empty the "add" row input value
this.clearSearch();
}

searchData() {
return this.input.select2('data')
}

clearSearch() {
this.input.select2("val", '');
}

/**
* Set the warning message related to the appropriate row in the table
* @param {String} message the warning message text to set
*/
setWarningMessage(message) {
this.warning.text(message).removeClass("hidden");
}

/**
* Hide the warning message on the appropriate row
*/
hideWarningMessage(){
this.warning.addClass("hidden");
}
}
60 changes: 60 additions & 0 deletions app/assets/javascripts/sufia/relationships/registry.es6
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import RegistryEntry from './registry_entry'
export default class Registry {
/**
* Initialize the registry
* @param {jQuery} element the jquery selector for the permissions container
* @param {String} object_name the name of the object, for constructing form fields (e.g. 'generic_work')
* @param {String} templateId the the identifier of the template for the added elements
*/
constructor(element, objectName, propertyName, templateId) {
this.objectName = objectName
this.propertyName = propertyName

this.templateId = templateId
this.items = []
this.element = element

// the remove button is only on preexisting grants
element.find('[data-behavior="remove-relationship"]').on('click', (evt) => this.removeWork(evt))
}

// Return an index for the hidden field when adding a new row.
// This makes the assumption that all the tr elements represent a work except
// for the final one, which is the "add another" form
nextIndex() {
return this.element.find('tbody').children('tr').length - 1;
}

addWork(work) {
work.index = this.nextIndex()
this.items.push(new RegistryEntry(work, this, this.element.find('tr:last'), this.templateId))
this.showSaveNote();
}

// removes a row that has been persisted
removeWork(evt) {
evt.preventDefault();
let button = $(evt.target);
let container = button.closest('tr');
container.addClass('hidden'); // do not show the block
this.addDestroyField(container, button.attr('data-index'));
this.showSaveNote();
}

addDestroyField(element, index) {
$('<input>').attr({
type: 'hidden',
name: `${this.fieldPrefix(index)}[_destroy]`,
value: 'true'
}).appendTo(element);
}

fieldPrefix(counter) {
return `${this.objectName}[${this.propertyName}][${counter}]`
}

showSaveNote() {
// TODO: we may want to reveal a note that changes aren't active until the work is saved
}

}
38 changes: 38 additions & 0 deletions app/assets/javascripts/sufia/relationships/registry_entry.es6
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
export default class RegistryEntry {
/**
* Initialize the registry entry
* @param {Work} work the work to display on the form
* @param {Registry} registry the registry that holds this registry entry.
* @param {jQuery} element a place to insert the new row
* @param {String} template identifer of the new row template.
*/
constructor(work, registry, element, template) {
this.work = work
this.registry = registry
this.element = element

let row = this.createRow(work, template);
this.addHiddenField(row, work);
row.effect("highlight", {}, 3000);
}

// Remove a row that has not been persisted
createRow(work, templateId) {
let row = $(tmpl(templateId, work));
this.element.before(row);
row.find('[data-behavior="remove-relationship"]').click(function () {
row.remove();
});

return row;
}

addHiddenField(element, work) {
var prefix = this.registry.fieldPrefix(work.index);
$('<input>').attr({
type: 'hidden',
name: prefix + '[id]',
value: work.id
}).appendTo(element);
}
}

0 comments on commit fca4aef

Please sign in to comment.