Loading
bug: #3633409: Clean up the deferred findings of Mapillary organizations on Group
Summary
Fixes the findings that the reviews of #3631545 deferred, as listed in #3633409. They were checked again against the current 1.0.x before this work. Each item is its own commit, so the MR can be read commit by commit.
What changes
Access and caching
- The group page of a public organization is open to non-members. Members-only organizations stay closed, and only
viewis opened. - Update listings include the content of the organizations where the account may edit it, as delete listings already do.
- The unused group permission
add mapillary contentis removed. The description ofadminister mapillary organizationnow says what it covers. - The username field access is cached per user and per target user.
Assignment and queries
- An image added to an organization by hand brings its sequence, detections and map features.
- An account in no organization assigns nothing. Map features that list many images use the queue, like the other assignment paths.
- The organizations of an account (view access, importer) are kept for the request and refreshed when a tag they depend on is invalidated. The rows of the mismatched content tab look up their owner once.
Import
ImportGatealso accepts an image that is related to the organization already, so content added by hand can be imported again.- Site administrators keep the map feature option of the detection import form.
- A refused import deletes the sequence it created only when no other image uses it.
- The thumbnail refresh saves the Mapillary organization ID even without a usable thumbnail, and uses the image importer's check of that ID.
Update from 1.0.0-alpha6
- Content language settings, the components of every shared form and view mode, and the
third_party_settingsof site-built fields are carried over. - A rerun lists the organizations migrated without their ID again, and its assignment pass covers groups an earlier run created.
- The mismatched content route needs Group (404 before the update).
RelationshipTargetSchemaresizes only integer columns.- Uninstalling a Mapillary submodule revokes the permissions of its relation plugin from the organization roles.
Listings and texts
- The organization filter is a select list of the organizations the viewer may see. The ID of a hidden organization and a value such as
abcmatch nothing. - Notice texts use plural forms. The notice about a creator who moved names the organization of the old membership.
- "Member since" has a view display component. The membership form says when it cannot save a Mapillary username.
- Only the default display of a listing gets the handlers; the README says so.
Code and tests
- One helper for trimmed field values (
FieldValue) and one service for the Group check (GroupInstalled). The API call log inserts the organizations of a call in one query.imageQueries()andmemberUsernames()are marked@internal. - More test coverage (roles, grants, field access, members-only map features and detections, listing invalidation, another group type, rerun paths, site administrators, filter for outsiders and members) and a shared trait for the kernel test helpers.
- The two deprecations of the next major phpstan job are fixed.
Upgrade path
New post updates, all safe to rerun:
mapillary_organization_post_update_revoke_add_permissionmapillary_organization_post_update_replace_organization_filtermapillary_organization_post_update_member_since_view_display
The alpha6 update (mapillary_organization_update_11104) also copies the extra configuration listed above. The new post updates sort before uninstall_unused, which has to stay last.
API and data model changes
- The group permission
add mapillary contentis removed; the post update revokes it from the roles. - Constructors of internal services and hook classes changed (new
GroupInstalled, cache tags checksum, messenger). They are not public API. Noticehas a new optional$leftargument.
Notes for reviewers
- The two deprecations are replaced with calls that also exist on Drupal 11.2 (
getSession()->getPage()->findAll()andRole::grantPermission()), becausegetNodeElementsByXpath()does not exist there. - The thumbnail refresh commit covers two items of the issue, as they are the same block of code.
- Not in this MR: the cost of access checks per row. Preloading the relationships of a page of entities is a bigger change and can have its own issue.
- Testing: the tests of every touched class and the browser tests of the organization module were run locally on PostgreSQL. Full, MySQL and Drupal 12 runs are left to the pipeline.
Closes #3633409