ensure weights in integer weight select

[qadan]

What does this Pull Request do?

Fixes an issue where the integer weight selector can be made impossible to correctly re-weight things using the select menus.

The issue can be seen if you take the steps under "How should this be tested" below.

You'll see (in the current codebase) that performing the steps in the test case makes it so that you cannot change the weight values of the child members using the select fields.

This is happening because those select menus are populated by first populating options for each current weight value for each member, then if any weights are currently an empty string, running through and adding a number for each row, starting from 1.

It breaks because of two things:

• View rows actually start from 0, whereas the options are populated from 1, so there will always be one fewer number in the select menu than the total rows in the view
• After the first time you reorder things, and until you add a new member, there will not be an empty string triggering the second condition, so the only numbers in the list will be used as options for weights.

This is kind of compounded by the fact that if you do indeed allow someone to modify weights as part of the Repository Item creation form, they can put any number they'd like in there, so once you attempt reordering, there's no guarantee you'll have numbers in the select field you can use to reorder weights.

What's new?

The form now creates weight select fields that contain all existing values as options, plus one guaranteed option for each possible row number.

How should this be tested?

• Create a collection
• Add two repository items to that collection
• Use the "Reorder members" view to give one of the members a weight of 2 using the select drop-downs (might have to "Show row weights" first), then save the order
• Use the "Reorder members" view to then give the member you previously set at 2 a weight of 1
• Visit the "Reorder members" view again, and observe that both select fields allow you to reorder the items

Additional Notes:

This might not be the best solution to the issue ... weight fields in forms use the 'number' type form element, so would it be better to do so here as well? That would also ensure that any arbitrary number is still valid, works with #default_value, and doesn't create a massive select field.

Interested parties

@Islandora/8-x-committers

[mjordan]

I will test this.

[seth-shaw-unlv]

@qadan, we were starting the counter from one due to the semantics of children, e.g. (overly-simplified) a book has page 1, not page 0. Is there a way to fix this while still preserving the counter starting from 1?

Also, we placed that iterator inside the blank weight field check because we want to preserve the existing weight values. This is important because very large collections crash attempting to load all the children at once, so we need to support pagination. This way each page of members has its own range of values to work with (e.g. 11-20 for page two of a 10 items per page view).

[qadan]

Honestly I think the most correct way to do this would be to use the same type of render item that number fields actually use, which is the number selector that lets you type in a number but has up and down buttons to let you increment or decrement the number. The calculations required to render a useful drop-down menu would just kind of go away in that case. I'm not sure though, I guess it's up to what folks consider useful here? I can also pop in and address that feedback for sure though

[seth-shaw-unlv]

@qadan, I believe the select box is required by Drupal's built-in Drag-n-Drop reordering interface, although that is just an assumption.

[dannylamb]

Travis is fixed now. I've restarted the build to see if we can move on this and all the other PRs for islandora.

[dannylamb]

Hrm... I peeked at the Travis config through their UI and it looks like it's still using the old .travis.yml: https://travis-ci.org/github/Islandora/islandora/builds/732347040/config.

@qadan If you update from 8.x-1.x Travis should ™️ go green.