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

Deprecate annotation-specific properties in the Sluggable extension annotation/attribute classes #2837

Merged

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Jul 6, 2024

When attributes support was added, the way slug handlers were configured was changed between annotations and attributes. With annotations support now fully deprecated, it doesn't make sense to continue supporting annotation-specific config paths, so this PR deprecates the annotation style slug handler configuration. Specifically, this will:

  • Deprecate the Slug attribute's $handlers property, slug handlers are now configured through property-level attributes
  • Deprecate the SlugHandlerOption class entirely, it is unused by the attribute driver in favor of providing the options as an associative array on the SlugHandler attribute

Below example (taken right out of the docs) is the same configuration for a field using both annotations and attributes to give a visual reference.

/**
 * @ORM\Column(type="string", unique=true)
 * @Gedmo\Slug(
 *   fields={"title"},
 *   handlers={
 *     @Gedmo\SlugHandler(
 *       class="Gedmo\Sluggable\Handler\TreeSlugHandler",
 *       options={
 *         @Gedmo\SlugHandlerOption(name="parentRelationField", value="category"),
 *         @Gedmo\SlugHandlerOption(name="separator", value="/")
 *       }
 *     )
 *   }
 * )
 */
#[ORM\Column(type: Types::STRING, unique: true)]
#[Gedmo\Slug(fields: ['title'])]
#[Gedmo\SlugHandler(class: TreeSlugHandler::class, options: ['parentRelationField' => 'category', 'separator' => '/'])]
public ?string $slug = null;

doc/attributes.md Show resolved Hide resolved
src/Mapping/Annotation/Slug.php Outdated Show resolved Hide resolved
src/Mapping/Annotation/Slug.php Outdated Show resolved Hide resolved
@phansys phansys requested a review from franmomu July 25, 2024 00:06
Copy link

codecov bot commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.66%. Comparing base (0ad4856) to head (c19083f).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2837   +/-   ##
=======================================
  Coverage   78.66%   78.66%           
=======================================
  Files         167      167           
  Lines        8746     8746           
=======================================
  Hits         6880     6880           
  Misses       1866     1866           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mbabker mbabker force-pushed the deprecate-sluggable-annotation-props branch from 7ab89de to f55dd9c Compare July 25, 2024 14:04
@mbabker mbabker force-pushed the deprecate-sluggable-annotation-props branch from f55dd9c to 6ba82a1 Compare October 7, 2024 02:14
src/Mapping/Annotation/Slug.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@phansys phansys left a comment

Choose a reason for hiding this comment

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

Thanks @mbabker!

Could you please add an entry at CHANGELOG.md for these deprecations?

…nnotation/attribute classes

Co-authored-by: Javier Spagnoletti <phansys@gmail.com>
@mbabker mbabker force-pushed the deprecate-sluggable-annotation-props branch from d5ac53b to d8701ed Compare November 11, 2024 15:52
@mbabker
Copy link
Contributor Author

mbabker commented Nov 11, 2024

Could you please add an entry at CHANGELOG.md for these deprecations?

Added

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Javier Spagnoletti <phansys@gmail.com>
@phansys phansys merged commit 22418b1 into doctrine-extensions:main Nov 11, 2024
22 checks passed
@phansys
Copy link
Collaborator

phansys commented Nov 11, 2024

Thank you @mbabker!

@mbabker mbabker deleted the deprecate-sluggable-annotation-props branch November 11, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants