CommentFileSizeAuthor
#10 2880499-8.patch4.44 KBmahtab_alam
#7 2880499-7.patch826 bytesmahtab_alam
#7 2880499-7.patch822 bytesmahtab_alam
#2 content_access-add-migrate-support-2880499-2.patch4.18 KBosopolar

Issue fork content_access-2880499

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

osopolar created an issue. See original summary.

osopolar’s picture

osopolar
Primary language German
Location 🇩🇪 GER 🌐
CreditAttribution: osopolar commented
FileSize
content_access-add-migrate-support-2880499-2.patch4.18 KB

Content Access settings might be set with the help of an Event Subscriber subscribing to the event MigrateEvents::POST_ROW_SAVE, see attached patch.

Example on how to set the Content Access Settings for operation view (node view) in a migrate template (see also the PHPDoc for MigrationEventSubscriber::migratePostRowSave):

destination:
  plugin: entity:node
process:
 'content_access_settings/view':
   source: content_visibility
   plugin: static_map
   map:
     public:
       - anonymous
       - authenticated
       - administrator
     private:
       - authenticated
       - administrator

The default settings don't have to be defined in the migration template, it would be just enough to define the customized settings, as above, where only the $content_access_settings['view'] is defined. In this example the settings for update or delete will be received from the content types default content access settings. If the per node settings matches the default settings the per node settings aren't saved to the database (as it's not necessary).

osopolar’s picture

osopolar
Primary language German
Location 🇩🇪 GER 🌐
CreditAttribution: osopolar commented
Status: Active » Needs review
rwmanos’s picture

rwmanos CreditAttribution: rwmanos commented
Status: Needs review » Needs work 
PHP Fatal error:  Class 'Drupal\migrate\Event\MigrateEvents' not found in /8.5.4/modules/content_access/src/EventSubscriber/MigrationEventSubscriber.php on line 104

Patch in #2 needs some work to prevent those fatal errors.

osopolar’s picture

osopolar
Primary language German
Location 🇩🇪 GER 🌐
CreditAttribution: osopolar commented
Status: Needs work » Needs review

Where did you define your migration? In a custom module? If so, may you please add the following to the modules .info.yml file:

dependencies:
  - drupal:migrate

It's just a guess based on what I found searching for your error message. It looks similar to the issue the commerce migrate module had, see: #2912247: Fatal error MigrateEvents not found.

Otherwise we may need to outsource the code in a custom sub-module with dependency on migrate – or the error is related to something else.

rwmanos’s picture

rwmanos CreditAttribution: rwmanos commented

I assumed some migration dependency is missing but I'd expect this dependency to be added in the module by the patch itself.
The point is that this patch is for content_access and if you apply it, you are not able to enable the module any more.

In addition, I enabled all migration-related modules (i.e. migrate_upgrade migrate_plus migrate_tools migrate migrate_drupal) but I still cannot enable this module with this patch applied. The error is different this time:

exception 'ReflectionException' with message 'Class Drupal\content_access\EventSubscriber\MigrationEventSubscriber does not exist' in                                                               [error]
/drupal/8.5.4/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterEventSubscribersPass.php:30

I strongly believe this patch needs some work.

mahtab_alam’s picture

mahtab_alam CreditAttribution: mahtab_alam at Valuebound commented
FileSize
2880499-7.patch822 bytes
2880499-7.patch826 bytes

The last submitted patch, 7: 2880499-7.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 7: 2880499-7.patch, failed testing. View results

mahtab_alam’s picture

mahtab_alam CreditAttribution: mahtab_alam at Valuebound commented
Status: Needs work » Needs review
FileSize
2880499-8.patch4.44 KB

Status: Needs review » Needs work

The last submitted patch, 10: 2880499-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mahtab_alam’s picture

mahtab_alam CreditAttribution: mahtab_alam at Valuebound commented
Issue summary: View changes

Patch #10 is working but it is not passing the test because of drupal coding standards.

osopolar’s picture

osopolar
Primary language German
Location 🇩🇪 GER 🌐
CreditAttribution: osopolar commented

Adding the migrate dependencies directly to Content Access means that everybody who wants to use the Content Access module needs to enable the Migrate module, no mater if the site will use migration, or am I missing something?

It may be implemented in a submodule of Content Access (Content Access Migration) or in a custom module, which implements the migration if present. See also #5.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

Patch #10 also shouldn't specify @package Drupal\example_module\EventSubscriber.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

Also, is there a specific reason to rename MigrateEvents as MigrateEventsCore?

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: gisle at Hannemyr Nye Medier AS commented

Patch fails to apply, and needs to be rerolled against HEAD. Also comments needs to be addressed.

pattersonc made their first commit to this issue’s fork.

pattersonc opened merge request !5

pattersonc closed merge request !5

pattersonc opened merge request !6

pattersonc’s picture

pattersonc CreditAttribution: pattersonc as a volunteer commented
Issue summary: View changes

See MR #20 as I added proper migration support for per node content_access records.

nsciacca made their first commit to this issue’s fork.

nsciacca’s picture

nsciacca
She/Her/Hers
Primary language English
Location Connecticut
CreditAttribution: nsciacca commented

@pattersonc Thanks for your merge request - with a few tweaks I made it work well for my implementation. I pushed these changes:

  • Replaced "acpol_migrate" with "content_access" in class namespaces
  • Added $supportsRollback = TRUE and a rollback function calling "content_access_delete_per_node_settings"
  • Using migrate.lookup service to get the destination role id as mine had capitalizations and spaces that weren't working
  • Code standards checked
bisonbleu’s picture

bisonbleu CreditAttribution: bisonbleu commented

Can someone please explain what the setup is to make this work? Thanks!

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: gisle at Hannemyr Nye Medier AS commented
Version: 8.x-1.x-dev » 2.0.x-dev

This should go into the version compatible with Drupal 10.

sclsweb’s picture

sclsweb CreditAttribution: sclsweb commented

I need to migrate a D7 site using Content Access to Drupal 10 and would love to see this added to the D10 version. I can't code it, but I would test if someone ported it.