Issue #2170: Sort "manage members" View results by field_weight

[mjordan]

GitHub Issue: Islandora/documentation#2170

What does this Pull Request do?

Adds a sort (by field_weight, ascending) to the Manage Members View.

What's new?

Added a Sort Criteria to all displays in the Manage Members View, depicted in the View admin GUI as:

• Does this change add any new dependencies?

No

• Does this change require any other modifications to be made to the repository
  (i.e. Regeneration activity, etc.)?

No

• Could this change impact execution of existing code?

No

How should this be tested?

1. Check out this branch.
2. Create an object and a couple of children/member objects, assigning each a desired weight under the System fieldset in their add/edit form.
3. Click on the parent object's Members tab to see how they sort.
4. Modify the weight on one or more of the children.
5. Revisit the parent's Members tab. The children should sort using to their newly assigned weight values, in ascending order.

Documentation Status

• Does this change existing behaviour that's currently documented?

No

• Does this change require new pages or sections of documentation?

No

• Who does this need to be documented for?

n/a

Additional Notes:

I exported this View YAML using /admin/config/development/configuration/single/export (Drupal 9.4.3). There seem to be more changes in the YAML than I expected.

Interested parties

@Islandora/committers

[mjordan]

View config export failing in 9.5.x-dev. I'll try to update Drupal to that version and try again.

[mjordan]

Which, I'm not surprised to find out, is not as easy as https://www.drupal.org/project/drupal/releases/9.5.x-dev makes it sound:

composer require drupal/core-recommended:9.5.x-dev@dev drupal/core-composer-scaffold:9.5.x-dev@dev drupal/core-project-message:9.5.x-dev@dev --update-with-all-dependencies
./composer.json has been updated
Running composer update drupal/core-recommended drupal/core-composer-scaffold drupal/core-project-message --with-all-dependencies
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/core-recommended 9.5.x-dev@dev -> satisfiable by drupal/core-recommended[9.5.x-dev].
    - drupal/core-recommended 9.5.x-dev requires drupal/core 9.5.x-dev -> found drupal/core[9.5.x-dev] but it does not match your minimum-stability.


Installation failed, reverting ./composer.json and ./composer.lock to their original content.

Anybody got any suggestions on how to proceed?

[rosiel]

Must be the new composer version that was causing problems in the playbook, because I'm sure this worked before.

• Fix composer invocation to _not_ run as root. Islandora-Devops/islandora-playbook#227

These do not block merging, so I'm ok to merge anyway and we take this into a different issue?

[rosiel]

Must be the new composer version that was causing problems in the playbook, because I'm sure this worked before.

* [Fix composer invocation to _not_ run as root. Islandora-Devops/islandora-playbook#227](https://github.com/Islandora-Devops/islandora-playbook/pull/227)

These do not block merging, so I'm ok to merge anyway and we take this into a different issue?

I got this completely wrong.

If you're working with an existing root Drupal composer.json, then you probably need to set minimum-stability: dev in order for your upgrade from a stable version to a dev version to work. The Drupal recommended-project repo gives you a minimum-stability appropriate to whether you're on a branch or a tag (on the 9.N.x branches, the minimum-stability is dev, but if you get any tag e.g. 9.4.2, then the minimum-stability is stable.) So a 'fresh' install going to 9.5.x doesn't have to worry about the stability problem that you ran into.

That said... CI does a 'fresh' install and its problems were not the same as your problem... (I wrongly assumed it was). The CI is running into this:

The following module(s) will be enabled: islandora_audio, islandora_breadcrumbs, islandora_iiif, islandora_image, islandora_video, islandora_text_extraction_defaults, islandora, jsonld, hal, serialization, rdf, search_api, jwt, key, rest, filehash, basic_auth, context_ui, context, action, eva, media, prepopulate, features_ui, features, config_update, migrate_source_csv, migrate, content_translation, language, flysystem, token, file_replace, islandora_core_feature, migrate_plus, islandora_text_extraction

 // Do you want to continue?: yes.                                              

Error: ]  TypeError: Argument 1 passed to Drupal\Component\DependencyInjection\Container::generateServiceIdHash() must be an object, string given, called in /opt/drupal/web/core/lib/Drupal/Component/DependencyInjection/ServiceIdHashTrait.php on line 19 in Drupal\Component\DependencyInjection\Container->generateServiceIdHash() (line 28 of /opt/drupal/web/core/lib/Drupal/Component/DependencyInjection/ServiceIdHashTrait.php) #0 /opt/drupal/web/core/lib/Drupal/Component/DependencyInjection/ServiceIdHashTrait.php(19): Drupal\Component\DependencyInjection\Container->generateServiceIdHash()

Something is not doing dependency injection right. I can't tell if it's our code, or if the 9.5.x-dev branch did an oops. I'll see if i can install and test.

[mjordan]

I could revise my PR to just add the new sort YAML (https://github.com/Islandora/islandora/pull/900/files#diff-75da1677d22de691f5278bdeb8be7c26971d9e4492f7981ee093fb90a4116ef2R218-R231) to the existing View config YAML and see if that passes, if you want.

[whikloj]

See #899 (comment) for my diagnosis of this problem.

[rosiel]

Your code seems fine to me (and a reminder that we should update our core configs more often). [edit to add: the problem seems to be within drupal 9.5. I did a site update and replicated the error any time i tried to commit anything to the database)

This code change (and the testing procedure) does lead me to ask: Should we create an update hook for this, which would remove any edits one may have made to the Members list? (I would imagine that I'd at least add thumbnails if i had a Members list)

[mjordan]

Should we create an update hook for this, which would remove any edits one may have made to the Members list? (I would imagine that I'd at least add thumbnails if i had a Members list)

I think a change in the View config YAML only applies on module install, so existing local mods to the Manage Members View would not be touched unless someone uninstalled, then reinstalled the features module. Could be wrong about that however.

If that's correct, I'm still willing to try adding the new sorts YAML to the existing YAML config data. We wouldn't need to merge if it did and thought there were unknown risks like the one you're describing @rosiel .

[rosiel]

I think a change in the View config YAML only applies on module install, so existing local mods to the Manage Members View would not be touched unless someone uninstalled, then reinstalled the features module. Could be wrong about that however.
Yep, this is my interpretation too. I was being ... me... and bringing up a larger and out-of-scope question, regarding whether we maybe should go out of our way to make an "update path" for existing users. We've done a few hook_update implementations but only to fix bugs, not to add new features.

I'm still willing to try adding the new sorts YAML to the existing YAML config data.

As you wish. I'm ok with the diff as it is. I don't think it's causing the errors. IIRC @whikloj suggested restui and openseadragon, with this recent change to 9.5.x, might be the problem.

[mjordan]

OK, let's leave the PR as is. Is the plan to merge without 9.5.x-dev passing?

[seth-shaw-asu]

@rosiel , do you have permission to force the merge despite the failing tests, or do you need someone else to?