Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renames entity_bundle to islandora_entity_bundle to avoid name collision #772

Merged
merged 6 commits into from
Apr 13, 2020

Conversation

dannylamb
Copy link

@dannylamb dannylamb commented Apr 8, 2020

JIRA Ticket: Islandora/documentation#1470

Supersedes #771

What does this Pull Request do?

Changes the plugin_id for the entity_bundle Condition to islandora_entity_bundle to avoid a name collision with ctools entity_bundle plugin.

It also has an update hook to update any contexts that may be using the entity_bundle condition.

When this patch is applied, it should be possible to install pathauto and ctools and generate url aliases for taxonomy terms in the corporate_body vocabulary.

How should this be tested?

  1. On a fresh install, download & enable pathauto (also brings ctools).
  2. Visit /admin/config/search/path/patterns/add and try and specify a pathauto pattern for a corporate_body term, e.g. pattern=/agent/[term:name]. Observe "The website encountered an unexpected error. Please try again later." or "Drupal\Component\Plugin\Exception\PluginNotFoundException: The "entity_bundle:taxonomy_term" plugin does not exist." errors.
  3. Go visit admin/structure/context and get a whitescreen
  4. Apply this PR.
  5. Run the update hook by executing drush updatedb from the command line.
  6. Re-try defining pathauto pattern. This time creating a pathauto pattern should succeed.
  7. Go visit admin/structure/context. You should not whitescreen

Interested parties

@Islandora/8-x-committers

@dannylamb dannylamb changed the title Kayakr 8.x 1.x Renames entity_bundle to islandora_entity_bundle to avoid name collision Apr 8, 2020
@dannylamb
Copy link
Author

@kayakr Can you confirm this still works for you?

@Islandora/8-x-committers Anyone want to attempt running this update hook to make sure we're good?

@dannylamb
Copy link
Author

Ok, that last commit should make travis happy

@dannylamb
Copy link
Author

🎉 Travis is happy!

@kayakr
Copy link

kayakr commented Apr 9, 2020

@dannylamb I realise the branch I built the original patch in (#771) had a commit that's already been applied to 8.x-1.x (for derivative term), so I removed that.

Now the patch applied and fixes the issue.

The db update hook fails.

 ----------- ----------- --------------- ------------------------------------- 
  Module      Update ID   Type            Description                          
 ----------- ----------- --------------- ------------------------------------- 
  islandora   8002        hook_update_n   Replaces 'entity_bundle' conditions  
                                          with 'islandora_entity_bundle'.      
                                          This prevents plugin naming          
                                          collisions between islandora and     
                                          ctools.                              
 ----------- ----------- --------------- ------------------------------------- 

 // Do you wish to run the specified pending updates?: yes.                                                             

>  [notice] Update started: islandora_update_8002
>  [error]  You have requested a non-existent service "plugin.manager.context". 
>  [error]  Update failed: islandora_update_8002 

Should this be plugin.manager.context_reaction instead?

Do we still need a matching patch for the context definition in islandora_defaults.

@dannylamb
Copy link
Author

Yes, we will need a matching islandora_defaults PR. I can whip that up.

Not sure what's up with that service not being present tho. I was testing in pieces so let me try again from a fresh box and see what's up.

@dannylamb
Copy link
Author

Gah, it should be condition, not context. I made that error when testing and somehow it snuck in. One too many undo's...

@dannylamb
Copy link
Author

islandora_defaults PR is up ^^

Copy link

@kayakr kayakr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dannylamb, working for me.

@whikloj
Copy link
Member

whikloj commented Apr 13, 2020

Testing

Copy link
Member

@whikloj whikloj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright this does not cause a WSOD for me with any context reaction, but it does block the use of pathauto as described and it is the right choice for naming.

@whikloj whikloj merged commit 7b24912 into Islandora:8.x-1.x Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants