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

M3: Refactor Deprecations - ApiBundle #7985 #8057

Merged
merged 10 commits into from Nov 27, 2019
Merged

M3: Refactor Deprecations - ApiBundle #7985 #8057

merged 10 commits into from Nov 27, 2019

Conversation

mtshaw3
Copy link
Contributor

@mtshaw3 mtshaw3 commented Nov 1, 2019

PR for #7985

ApiBundle/Form/Validator/Constraints/OAuthCallbackValidator.php

  • Nr. 350 line 52: Calling deprecated method Symfony\Component\Validator\ConstraintValidator->buildViolation() - NOTE: This one is confusing because all though it is marked deprecated in the code, the Symfony 3.0 upgrade doc says to use it????

ApiBundle/Form/Type/ClientType.php

  • Nr. 351 line 267: Using deprecated interface Symfony\Component\OptionsResolver\OptionsResolverInterface
  • Nr. 352 line 267: Overriding deprecated method Mautic\ApiBundle\Form\Type\ClientType->setDefaultOptions()
  • Nr. 353 line 280: Overriding deprecated method Mautic\ApiBundle\Form\Type\ClientType->getName()

ApiBundle/Form/Type/ConfigType.php

  • Nr. 354 line 117: Overriding deprecated method Mautic\ApiBundle\Form\Type\ConfigType->getName()

@npracht npracht added this to the 3.0.0 milestone Nov 1, 2019
@npracht npracht added Mautic 3 WIP PR's that are not ready for review and are currently in progress labels Nov 1, 2019
@escopecz
Copy link
Sponsor Member

escopecz commented Nov 1, 2019

@mtshaw3 check the Travis CI

@escopecz escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Nov 1, 2019
@mtshaw3 mtshaw3 removed the WIP PR's that are not ready for review and are currently in progress label Nov 19, 2019
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

@mtshaw3 We are building a form in Mautic\ApiBundle\Form\Type\ConfigType.
Could you please pass fully qualified class name (FQCN) to the add method as described here:
https://github.com/symfony/symfony/blob/3.0/UPGRADE-3.0.md#form ?
Same thing applies for Mautic\ApiBundle\Form\Type\ClientType class.

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

I wonder how to test this. Do you have any ideas? :)
It's not practically possible to test every api call. I'm going to test only couple of them.
Is there anything else that I have to pay attention to?

app/bundles/ApiBundle/Form/Type/ConfigType.php Outdated Show resolved Hide resolved
app/bundles/ApiBundle/Config/config.php Show resolved Hide resolved
@anton-vlasenko anton-vlasenko added the code-review-needed PR's that require a code review before merging label Nov 22, 2019
@anton-vlasenko
Copy link
Contributor

I wonder how to test this. Do you have any ideas? :)
It's not practically possible to test every api call. I'm going to test only couple of them.
Is there anything else that I have to pay attention to?

@mtshaw3 I think that's a brilliant idea of yours to test API bundle using our api library tests.
I will do it.

@escopecz escopecz added ready-to-test PR's that are ready to test and removed pending-feedback PR's and issues that are awaiting feedback from the author labels Nov 25, 2019
@dongilbert
Copy link
Member

Tried testing with the API-Library. Every endpoint is giving me a 404 error. I've ensured the API is enabled, and I'm using HTTP Basic Auth.

@dongilbert
Copy link
Member

Was able to get the API library tests running - will post results when they finish

@anton-vlasenko
Copy link
Contributor

The code looks fine.
I've tested this and the unit tests showed no new errors.
Therefore I approve this PR.

@anton-vlasenko
Copy link
Contributor

@dongilbert Would you like to add your own review?
If not, please let me know so I could close this PR.

@dongilbert dongilbert merged commit b410104 into mautic:m2-to-m3 Nov 27, 2019
@escopecz escopecz removed code-review-needed PR's that require a code review before merging ready-to-test PR's that are ready to test labels Nov 27, 2019
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

5 participants