Api fixes #8413
Merged
Api fixes #8413
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
escopecz
added
Mautic 3
regression
|
@escopecz Feel free to mention me when this PR is done, I have the API library set up for testing 👍 |
|
@dennisameling it's ready. But also check out the API Library 3.0 branch before you run the tests as it has deprecated methods removed. |
There was a problem hiding this comment.
@escopecz I'm still getting a few failures.
Testing environment
- DDEV PHP 7.2
- Mautic instance is based on https://github.com/escopecz/mautic/tree/api-fixes branch
- API library is based on the https://github.com/mautic/api-library/tree/3.0 branch
- API enabled in Mautic config + using Basic Auth
CLICK ME
Time: 9.28 minutes, Memory: 8.00 MBThere were 9 failures:
- Mautic\Tests\Api\ContactsTest::testEditPatch
The response has unexpected status code (500).Response: {"errors":[{"message":"PHP Notice - Undefined index: eventName","code":500,"type":null}],"trace":[{"namespace":"","short_class":"","class":"","type":"","function":"","file":"/var/www/html/app/bundles/PointBundle/Model/TriggerModel.php","line":350,"args":[]},{"namespace":"Mautic\CoreBundle\ErrorHandler","short_class":"ErrorHandler","class":"Mautic\CoreBundle\ErrorHandler\ErrorHandler","type":"-\u003E","function":"handleError","file":"/var/www/html/app/bundles/PointBundle/Model/TriggerModel.php","line":350,"args":[["integer",8],["string","PHP Notice - Undefined index: eventName"],["string","/var/www/html/app/bundles/PointBundle/Model/TriggerModel.php"],["integer",350],["array",{"event":["array",{"id":["integer",57],"name":["string","tag test event"],"type":["string","lead.changetags"],"properties":["array",{"add_tags":["array",[["string","tag-a"]]]}],"trigger":["array",{"id":["integer",27],"name":["string","test"],"points":["integer",5],"color":["string","4e5...
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/ContactsTest.php:342
- Mautic\Tests\Api\ContactsTest::testAddPoints
The response has unexpected status code (400).Response: {"errors":[{"code":400,"message":"PHP Notice - Undefined index: eventName","details":[]}]}
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/ContactsTest.php:380
- Mautic\Tests\Api\EmailsTest::testSendToSegment
Failed asserting that 1 matches expected 0./var/www/html/tests/Api/EmailsTest.php:231
- Mautic\Tests\Api\EmailsTest::testSendToContact
Response does not contain success => true
Failed asserting that true is false./var/www/html/tests/Api/MauticApiTestCase.php:94
/var/www/html/tests/Api/EmailsTest.php:263
- Mautic\Tests\Api\MessagesTest::testGetListOfSpecificIds
The response has unexpected status code (400).Response: {"errors":[{"code":400,"message":"sms: This form should not contain extra fields.","details":{"sms":["This form should not contain extra fields."]}}]}
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/MauticApiTestCase.php:178
/var/www/html/tests/Api/MessagesTest.php:97
- Mautic\Tests\Api\MessagesTest::testCreateGetAndDelete
The response has unexpected status code (400).Response: {"errors":[{"code":400,"message":"sms: This form should not contain extra fields.","details":{"sms":["This form should not contain extra fields."]}}]}
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/MauticApiTestCase.php:237
/var/www/html/tests/Api/MessagesTest.php:104
- Mautic\Tests\Api\MessagesTest::testEditPatch
The response has unexpected status code (400).Response: {"errors":[{"code":400,"message":"sms: This form should not contain extra fields.","details":{"sms":["This form should not contain extra fields."]}}]}
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/MauticApiTestCase.php:104
/var/www/html/tests/Api/MessagesTest.php:74
/var/www/html/tests/Api/MauticApiTestCase.php:309
/var/www/html/tests/Api/MessagesTest.php:112
- Mautic\Tests\Api\MessagesTest::testEditPut
The response has unexpected status code (400).Response: {"errors":[{"code":400,"message":"sms: This form should not contain extra fields.","details":{"sms":["This form should not contain extra fields."]}}]}
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/MauticApiTestCase.php:104
/var/www/html/tests/Api/MessagesTest.php:74
/var/www/html/tests/Api/MauticApiTestCase.php:321
/var/www/html/tests/Api/MessagesTest.php:120
- Mautic\Tests\Api\MessagesTest::testBatchEndpoints
sms: This form should not contain extra fields.; sms: This form should not contain extra fields.; sms: This form should not contain extra fields.
Failed asserting that false is true./var/www/html/tests/Api/MauticApiTestCase.php:89
/var/www/html/tests/Api/MauticApiTestCase.php:104
/var/www/html/tests/Api/MessagesTest.php:74
/var/www/html/tests/Api/MauticApiTestCase.php:266
/var/www/html/tests/Api/MessagesTest.php:126
FAILURES!
Tests: 312, Assertions: 5363, Failures: 9, Skipped: 4.
|
@dennisameling thanks! I'll look at those. Could you please enable the Twilio plugin? No credentials are needed. Just enable it. It should fix some SMS errors as the API endpoints are not available unless some SMS plugin is enabled. |
4 tasks
|
@escopecz Thanks, after enabling the Twilio plugin all MessagesTest tests pass. Then we just have the |
|
I rebased to fix the conflict. Also fixed point triggers. The change tag call back was removed together with form events refactoring. Fixed it by adding the handling as a new event instead of adding back the static method. @dennisameling could you please run it once more? Also, please double check before that your testing Mautic instance can send emails. |
…orrectly determined which means API fields were not processed correctly
… when a model short key like `point.triggerEvent` is not found.
… create API response
The \Mautic\LeadBundle\Helper\EventHelper::updateTags callback was removed in PR 8186 as it was used with form event as well. Using the event system instead of adding the static callback.
…e (like birthday)
|
Might be able to check tomorrow but will be focused on the 2.16.0 release as well. If I start testing I'll drop a comment here before I actually start. But it's always a good thing if multiple people test it! |
|
I was testing with configlogEDIT: I'm getting responses via Postman well when follow redirects enabled. @escopecz It seems that HTTPS is now problem only in API library maybe with self signed cert |
|
Result from calling via HTTP requests Tests: 312, Assertions: 5086, Failures: 15, Skipped: 4. |
|
@hluchas Can you post what error you have in the logs for the failed |
Webhooks: Column 'secret' cannot be null And after ContactsTest run I got: |
Merged
|
I found out why I'm not getting the webhook errors. It's because the migration set default NULL but if the database schema is generated from the entities it's not a nullable column. The migration must allow null because it generates the secret keys for existing webhooks afterwards. I know how to replicate now, so working on a fix. |
… webhook via API.
|
Ad ContactsTest. I'm getting but when I used on target instance, tests are passing. It means that tests are dependent on specific IDs and may cause duplicate errors. |
hluchas
approved these changes
Feb 17, 2020
escopecz
commented
Feb 17, 2020
| 'callback' => '\Mautic\LeadBundle\Helper\EventHelper::updateTags', | ||
| ]; | ||
| $event->addEvent('lead.changetags', $action); | ||
| public function onPointTriggerExecuted(TriggerExecutedEvent $event): void |
There was a problem hiding this comment.
Oops, bad method name. Should be onTriggerExecute. Will create new PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please be sure you are submitting this against the staging branch.
Description:
Running the API Library's functional test discovered many bugs. Fixing them here.
Steps to reproduce the bug:
Steps to test this PR: