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

[DRUP-803] Prevent installation alongside of the `apigee_edge_apidocs… #7

Merged
merged 3 commits into from May 24, 2019
Merged

[DRUP-803] Prevent installation alongside of the `apigee_edge_apidocs… #7

merged 3 commits into from May 24, 2019

Conversation

Jaesin
Copy link
Contributor

@Jaesin Jaesin commented May 22, 2019

…` module.

Copy link
Contributor

@jacine jacine left a comment

Choose a reason for hiding this comment

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

I just tested this, with Apigee Edge at apigee/apigee-edge-drupal@6cfe266 and it worked great.

DRUP-803

@Jaesin If you can make that name change real quick, that would be good...Otherwise, I think this is good to go.

return [
'apigee_api_catalog_module_conflict' => [
'title' => t('Apigee API Catalog'),
'description' => t('The "Apigee Edge Apidocs" module is currently installed. It is deprecated and should be uninstalled before installing the Apigee API Catalog module.'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to name this the same way the module does.

-         'description' => t('The "Apigee Edge Apidocs" module is currently installed. It is deprecated and should be uninstalled before installing the Apigee API Catalog module.'),
+         'description' => t('The "Apigee Edge API Docs" module is currently installed. It is deprecated and should be uninstalled before installing the Apigee API Catalog module.'),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I pushed a fix for this.

@jacine
Copy link
Contributor

jacine commented May 23, 2019

I'm testing out @arshad's patch at apigee/apigee-devportal-kickstart-drupal#114, while still on this branch (with apigee edge apidocs still) and this is what I'm seeing:

m10n kickstart demo_core_install php_rewrite=ok profile=apigee_devportal_kickstart langcode=en

@Jaesin
Copy link
Contributor Author

Jaesin commented May 23, 2019

Nice catch. I just pushed what I hope will fix that.

@jacine
Copy link
Contributor

jacine commented May 23, 2019

That fixed it! Thanks! 👍

@Jaesin
Copy link
Contributor Author

Jaesin commented May 23, 2019

The scenario that hasn't' been tested at this point is trying to install the apigee_api_catalog module with drush 9 while the apigee_edge_apidocs is already installed.

@Jaesin
Copy link
Contributor Author

Jaesin commented May 23, 2019

I installed drush 9 and it really has no respect for hook_requirements as mentioned in apigee/apigee-edge-drupal#198 (comment). If you have the apigee_edge_apidocs module installed, it will still install the apigee_api_catalog module without complaint which will result in the following error when visiting a page on the site:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Symfony\Component\Routing\Exception\RouteNotFoundException</em>: Route &quot;entity.apidoc.settings&quot; does not exist.

Should we work on figuring out a workaround for this?

@cnovak
Copy link
Collaborator

cnovak commented May 24, 2019

I added separate issue #10 so we can accept this PR w/out dealing w/Drush.

@Jaesin Jaesin merged commit 4adccc1 into apigee:8.x-1.x May 24, 2019
@Jaesin
Copy link
Contributor Author

Jaesin commented May 24, 2019

Fixes: #5

@Jaesin Jaesin deleted the DRUP-803_prevent-module-conflict branch May 24, 2019 18:47
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.

None yet

3 participants