Issue #3388012: Deprecation reports to be compatible with D10 #3533
Conversation
vcsvinicius
added
type: update
status: needs review
|
Thanks for contributing towards Open Social! A maintainer from the @goalgorilla/maintainers group might not review all changes from all teams/contributors. Please don't be discouraged if it takes a while. In the meantime, we have some automated checks running and it might be that you will see our comments with some tips or requests to speed up the review process. 😊 |
|
Codewise it looks good, but guess we need to functionally test out all the new access checks to verify it still works as intended. Phpstan failing with the baseline, that we still need to adjust. |
| @@ -160,14 +160,19 @@ function _social_follow_tag_update_existing_messages(array &$sandbox): void { | |||
| // Get all message IDs than should be updated. | |||
| $query = $message_storage->getQuery() | |||
| ->condition('template', 'create_node_following_tag_stream'); | |||
| $mids = $query->execute(); | |||
| $mids = $query | |||
| ->accessCheck(TRUE) | |||
There was a problem hiding this comment.
We can put this accessCheck to false, as when run it will be anonymous and only takes what the anonymous user can view. In this case we want to update this message template regardless to if the current user has access to it.
| $sandbox['total'] = $query->count()->execute(); | ||
| $sandbox['total'] = $query | ||
| ->count() | ||
| ->accessCheck(TRUE) |
|
When functionally testing it out it also noticed (not caused by this code) the whole social_follow_tag update hook code is not really working. It tries to update a message template "create_node_following_tag_stream" in a sandbox environment, while it's only 1 message template that it updates, so no sandbox needed. Also this message template does not exist on new installs as it's only added in an update hook. I will create a seperate issue for that. |
|
Btw @vcsvinicius don't forget to fill in the "release notes" part and maybe also the "change records" to tell them some of the deprecations will be removed in 13.0 and not 12.0 |
Open Social version 12 will only contain a Drupal 10 upgrade as its scope; this is why we are changing the deprecation of this method from version 12 to 13.
…social module and social submodule
Access checking must be explicitly specified on content entity queries see: https://www.drupal.org/node/3201242
Open Social version 12 will only contain a Drupal 10 upgrade as its scope; this is why we are changing the deprecation of this method from version 12 to 13.
Open Social version 12 will only contain a Drupal 10 upgrade as its scope; this is why we are changing the deprecation of this method from version 12 to 13.
Hook install are user agnostic, hence we don’t need to add accessCheck(true)
vcsvinicius
force-pushed
the
issue/3388012-Deprecation-reports-to-be-compatible-with-d10
branch
from
357025d
to
bf8cf7e
Compare
vcsvinicius
removed
the
status: needs review
robertragas
deleted the
issue/3388012-Deprecation-reports-to-be-compatible-with-d10
branch
robertragas
added
the
backport: verified
|
Approved, merged and cherry-picked to 12.0.x |
Kingdutch
added
type: dependencies
Problem
The Upgrade Status module, gave to us a report with all issues to be fixed to make the Open Social compatible with Drupal 10.
Solution
Social deprecation-function:
The major version 12 will be used to upgrade the Drupal to version 10, so all deprecation function from Social should be changed to be removed at major version 13.
Drupal deprecation-function:
Update all deprecated function from Drupal to the new function.
Sub-modules core version:
Update the drupal-core-version at info files from sub-modules.
Issue tracker
Theme issue tracker
N/A
How to test
Definition of done
Before merge
After merge
Screenshots
N/A
Release notes
Open Social version 12 will only contain a Drupal 10 upgrade as its scope; this is why we are changing the deprecation of this method from version 12 to 13.
Files with deprecated message update:
Change Record
N/A
Translations
N/A