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

[#319] Refactor add credit module configuration. #323

Merged
merged 3 commits into from
Feb 28, 2020

Conversation

arlina-espinoza
Copy link
Contributor

This PR needs apigee/apigee-m10n-drupal#223 (Compatibility with commerce 8.x-2.16), and together fix #319 .
The Apigee Kickstart Monetization submodule of Kickstart (apigee_kickstart_m10n_add_credit) is a configuration only module, and it is meant to be enabled during the initial Drupal installation, in the monetization step if the "Enable Add Credit module" is checked. If so, the Kickstart installer provided supplemental initialization in code. If the module is enabled after the installer completed, the other logic isn't run.
To solve this issue, efforts were made in the apigee_m10n module to allow a simpler configuration post-installation, and apigee_m10n 8.x-1.3 includes a way to configure the "Add credit" module in a simple way from a requirements page at: /admin/reports/requirements

  • This PR leverages that new feature in apigee_m10n during the install phase if "Enable Add Credit module" is checked.
  • This PR also deprecates the apigee_kickstart_m10n_add_credit submodule, as it's no longer needed, and removes the confusion of having a module that should not be installed after the initial installation.
  • We need a follow up ticket to call attention to the apigee_m10n_add_credit configuration page at: /admin/reports/requirements

@drupalninja: To test:

Copy link
Collaborator

@cnovak cnovak left a comment

Choose a reason for hiding this comment

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

Added comments. Do we have instructions/issue to track doc changes that need to be done?
I believe we need:

  1. Kickstart docs: Update instructions for setting up monetization during installer
  2. Kickstart docs: Update/create how to install monetization after installing if not done during installer
  3. M10n docs: Update/create Steps needed to configure m10n

Can we add a ticket w/this info?

apigee_devportal_kickstart.install Show resolved Hide resolved
@@ -97,6 +98,27 @@ public static function createPaymentGateway(array $values, array &$context) {
$context['results']['$gateway'] = $gateway;
$context['message'] = t('Created a default payment gateway.');
}
catch (\Exception $exception) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

See message above, createPaymentGateway is not being called, but should be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :)

@arlina-espinoza
Copy link
Contributor Author

@cnovak I've addressed the changes, could you review again please?

@arlina-espinoza
Copy link
Contributor Author

Added documentation issues: apigee/apigee-m10n-drupal#226 and #330.

Copy link
Collaborator

@cnovak cnovak left a comment

Choose a reason for hiding this comment

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

LGTM

apigee_devportal_kickstart.install Show resolved Hide resolved
@shadcn shadcn merged commit 57e8d2f into apigee:8.x-1.x Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates CLA has been signed
Projects
None yet
4 participants