strapi-community / strapi-plugin-slugify

A plugin for Strapi Headless CMS that provides the ability to auto slugify a field for any content type.
https://market.strapi.io/plugins/strapi-plugin-slugify
MIT License
45 stars 21 forks source link

Bug: SlugifyWithCount will continue to increase the count when saving models via the Admin #68

Closed selected-pixel-jameson closed 2 years ago

selected-pixel-jameson commented 2 years ago

I believe this pull request should resolve this issue.

Note this will prevent the slug from changing at all once it's been generated, which is how this should work in my opinion. Once a slug has been generated the only way it should be able to be updated is manually since it could result in 404 errors if a redirect is not put in place.

I've tested it when updating the referenced fields and when creating a new entity and it seems to be working correctly.

This is the referenced issue. https://github.com/ComfortablyCoding/strapi-plugin-slugify/issues/60

ComfortablyCoding commented 2 years ago

@selected-pixel-jameson thanks for the PR.

I would rather this be dependant on a plugin setting than be the default behaviour. If it's added as is that will need to be a major release as it breaks existing behaviour.

Not sure on the setting name yet, freezeSlug maybe?

selected-pixel-jameson commented 2 years ago

I've never worked with slugs in a CMS where the slug changes when the referenced field is updated. To me this seems more like a bug than a feature. Just imagine the end-user issues that would happen if the URL to a webpage changed every time a Wordpress author made a change to the title of an article. The internet would just be full of 404 pages. To me this is the expected behavior and it should be the default.

In any case I'll make that update. At this point I just need this issue resolved as it's preventing us from being able to make any updates via the Strapi Admin interface.

ComfortablyCoding commented 2 years ago

Appreciate it.

I will look into this further for the next major version. If I see this is the case for majority then I will remove the update functionality altogether. Makes things easier :)

selected-pixel-jameson commented 2 years ago

@ComfortablyCoding I'm not super experience with Strapi. I'm working on testing this and making updates, but I'm running into an issue because I need to make the slugify call in the services async slugify and this is causing issues.

It seems likes this is called within the bootstrap.js

SUPPORTED_LIFECYCLES.forEach((lifecycle) => {
        subscribe[lifecycle] = (ctx) => {
            getPluginService(strapi, 'slugService').slugify(ctx);
        };
    });

But I'm not sure how I would modify this to make it handle async calls.

selected-pixel-jameson commented 2 years ago

@ComfortablyCoding I've submitted my updates. There is now a config setting for updateSlugs which defaults true. The updates will prevent the slug from being updated if the referenced fields have not been altered. You can set the updateSlugs settings to false to prevent slugs from automatically being updated when the referenced fields are changed.

ComfortablyCoding commented 2 years ago

Thanks, will have a look at it.

ComfortablyCoding commented 2 years ago

Thanks!