Skip to content
Snippets Groups Projects

Issue #3396062: Deprecate NodeStorage

Closes #3396062

Merge request reports

Members who can merge are allowed to add commits.
Code Quality is loading
Test summary results are being parsed
Metrics reports are loading
Ready to merge by members who can write to the target branch.
  • The source branch is 39 commits behind the target branch.
  • 1 commit will be added to 11.x.
  • Source branch will be deleted.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 31 31 * The plugin ID for the plugin instance.
    32 32 * @param mixed $plugin_definition
    33 33 * The plugin implementation definition.
    34 * @param \Drupal\node\NodeStorageInterface $node_storage
    34 * @param \Drupal\Core\Entity\EntityStorageInterface $node_storage
    35 35 * The node storage.
    36 36 */
    37 public function __construct(array $configuration, $plugin_id, $plugin_definition, NodeStorageInterface $node_storage) {
    37 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityStorageInterface $node_storage) {
  • Adam Bramley added 1 commit

    added 1 commit

    Compare with previous version

  • Sascha Grossenbacher
  • Adam Bramley added 44 commits

    added 44 commits

    Compare with previous version

  • 514 516 #[Hook('configurable_language_delete')]
    515 517 public function configurableLanguageDelete(ConfigurableLanguageInterface $language): void {
    516 518 // On nodes with this language, unset the language.
    517 \Drupal::entityTypeManager()->getStorage('node')->clearRevisionsLanguage($language);
    519 $table = \Drupal::entityTypeManager()->getStorage('node')->getRevisionTable();
    • Hadn't notice this before.

      This is exactly the kind of methods that should be in a custom storage class. Stuff that directly interacts with the database and bypasses entity query. It's also not test code.

      It's not a solution to just do it outside of storage.

      However, it also shouldn't be a node specific thing, because any translatable + revisionable entity type has this issue, it being node specific is just a leftover from when only nodes had revisions.

      so either we do it for all of them or none. I know there are issues open about problems when removing translations, I'm not sure if this has test coverage and if it even does what it says. What happens if you remove it, are there failing tests?

      Could skip this method and repurpose this to just remove the methods that are easy to do.

    • However, it also shouldn't be a node specific thing, because any translatable + revisionable entity type has this issue, it being node specific is just a leftover from when only nodes had revisions. Could skip this method and repurpose this to just remove the methods that are easy to do.

      Making this generic to all revisionable/translatable entity types is out of scope of this issue. IMO we should do it the other way - deprecate it on the storage class and create a follow up to handle this for all entity types.

    • Please register or sign in to reply
  • Adam Bramley added 38 commits

    added 38 commits

    Compare with previous version

  • Adam Bramley added 97 commits

    added 97 commits

    Compare with previous version

  • Adam Bramley added 350 commits

    added 350 commits

    Compare with previous version

  • Please register or sign in to reply
    Loading