- Issue created by @abhisekmazumdar
- 🇮🇳India abhisekmazumdar India
abhisekmazumdar → changed the visibility of the branch 3527905-implement-multi-instance-mautic to hidden.
- 🇮🇳India abhisekmazumdar India
abhisekmazumdar → changed the visibility of the branch 2.0.x to hidden.
- 🇮🇳India abhisekmazumdar India
abhisekmazumdar → changed the visibility of the branch 3527905-multi-instance-mautic-connection to hidden.
- 🇧🇪Belgium mallezie Loenhout
Some remarks.
When creating a connection, i've selected oauth, filled nothing in and clicked save, this gave a WSOD, since key and secret should be required in that case.
When adding some random input for oauth it crashes on save (with key and secret filled in).
The website encountered an unexpected error. Try again later. Error: Typed property Mautic\Auth\TwoLeggedOAuth2::$_expires must not be accessed before initialization in Mautic\Auth\TwoLeggedOAuth2->getAccessTokenData() (line 102 of /var/www/html/vendor/mautic/api-library/lib/Auth/TwoLeggedOAuth2.php). Drupal\mautic_api\MauticApiConnector->getOauthAccessToken() (Line: 270) Drupal\mautic_api\Form\MauticApiConnectionForm->submitForm() call_user_func_array() (Line: 105) Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 43) Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 589) Drupal\Core\Form\FormBuilder->processForm() (Line: 321) Drupal\Core\Form\FormBuilder->buildForm() (Line: 73) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 622) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183) Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76) Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53) Drupal\Core\StackMiddleware\Session->handle() (Line: 48) Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28) Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32) Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116) Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90) Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48) Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51) Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53) Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715) Drupal\Core\DrupalKernel->handle() (Line: 19)
Also with valid credentials this shows up
For basic auth, i can save the connection.
Can we add the status message on top, and show warning when connection is not correct, now it's quite hidden.
Can we rename oauth settings to public key and secret key, and provide info where to set this up in mautic. (at /s/credentials)
In the overview, the status column has no use, since i cannot disable / enable connection in the UI. So we can or remove that field, or add a ui toggle for it (i would remove it). In the overview it might make more sense in the status to show the result of the connection test.
Last thing, when creating basic auth or toggling between oauth setup in a credentials, make sure to clear the other config. So when oauth is selected clear and thus not write to config the basic auth settings and vice versa.
- 🇮🇳India abhisekmazumdar India
In the overview it might make more sense in the status to show the result of the connection test.
I didn't add that because the status is not stored in the entity(database). To obtain the status of an API, we need to load the entire entity and make request for status all the connection entities. This might become problematic as we start to have more connection entities in the overview listing.
I just replaced the status with the site URL, which makes it easier to identify.
- I made the error handling better.
- Clear value if the auth method is toggled
- Rename the oauth to the said public and secret keys
- moved the status message in the top and just shows error msg doesn't show WSOD
All the other MR inline feedback has also been resolved now.
- 🇧🇪Belgium mallezie Loenhout
Tried adding an oauth connection, when saving got.
Mautic\Exception\RequiredParameterMissingException: One or more required parameters was not supplied. Both clientKey and clientSecret required! in Mautic\Auth\TwoLeggedOAuth2->setup() (line 63 of /var/www/html/vendor/mautic/api-library/lib/Auth/TwoLeggedOAuth2.php). ReflectionMethod->invokeArgs() (Line: 60) Mautic\Auth\ApiAuth->newAuth() (Line: 255) Drupal\mautic_api\Form\MauticApiConnectionForm->submitForm() call_user_func_array() (Line: 105) Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 43) Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 589) Drupal\Core\Form\FormBuilder->processForm() (Line: 321) Drupal\Core\Form\FormBuilder->buildForm() (Line: 73) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 622) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183) Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76) Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53) Drupal\Core\StackMiddleware\Session->handle() (Line: 48) Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28) Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32) Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116) Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90) Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48) Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51) Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53) Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715) Drupal\Core\DrupalKernel->handle() (Line: 19)
- 🇧🇪Belgium mallezie Loenhout
Also thought of one more thing, is to move the config under webservices in the menu, instead of system.
- 🇮🇳India abhisekmazumdar India
New changes done:
- My bad, the check of the old and new secret wasn't behaving well. I break it down and added proper checks.
- Also moved the config under web services where it made more sense.
- Made improvements to the consistency for the "mautic api connection" to "Mautic API Connection".
- 🇧🇪Belgium mallezie Loenhout
Thanks.
I've fixed a bug when creating a new access token, when there was no already available in state.
We can only get the token data when it's initialized and valid.This fixes below error.
The website encountered an unexpected error. Try again later. Error: Typed property Mautic\Auth\TwoLeggedOAuth2::$_expires must not be accessed before initialization in Mautic\Auth\TwoLeggedOAuth2-
For consistency also changed path (since it's now under the web service config menu).
Also some small addition to show the auth type on the overview.
Other code looks good, so if you can do a quick check of my last commits, this is good to go.
- 🇮🇳India abhisekmazumdar India
Thank you for the detailed review and for fixing the bugs. I have now merged the MR.
Automatically closed - issue fixed for 2 weeks with no activity.