Skip to content

Conversation

@fernandom06
Copy link
Contributor

What this PR does / why we need it:

  • Check slug in front to show error if is neccessary
  • Default slug in backend if generated slug is empty

Before

image

After

image image

Which issue(s) this PR fixes:

Fixes #5532

Special notes for your reviewer:

I have created a function to generate the slug in the frontend to be able to check it before sending it to the backend, I don't know if I should use some library like in the backend

Testing

Tested locally, w/Chrome dev tools for mobile UX.

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

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

Since we already do basically this on the backend, I think it would be better to keep the logic there and throw an error that we catch on the frontend. That way we don't duplicate the slug logic in both the frontend and backend.

We could even do a custom error message like "unable to generate slug" which the frontend could read. We already have some error messages that we directly show the user using i18n on the backend (though not many).

e.g. from LDAP:

if self.user.password == "LDAP" or self.user.auth_method == AuthMethod.LDAP:
    raise HTTPException(
        status.HTTP_400_BAD_REQUEST, ErrorResponse.respond(self.t("user.ldap-update-password-unavailable"))
    )

This will automatically flash an error to the user on the frontend (we have a custom plugin for this).

@fernandom06 fernandom06 force-pushed the bugfix/5532-fix-dash-slug-names branch from c0f7438 to abc7237 Compare August 23, 2025 19:45
@fernandom06
Copy link
Contributor Author

Since we already do basically this on the backend, I think it would be better to keep the logic there and throw an error that we catch on the frontend. That way we don't duplicate the slug logic in both the frontend and backend.

We could even do a custom error message like "unable to generate slug" which the frontend could read. We already have some error messages that we directly show the user using i18n on the backend (though not many).

e.g. from LDAP:

if self.user.password == "LDAP" or self.user.auth_method == AuthMethod.LDAP:
    raise HTTPException(
        status.HTTP_400_BAD_REQUEST, ErrorResponse.respond(self.t("user.ldap-update-password-unavailable"))
    )

This will automatically flash an error to the user on the frontend (we have a custom plugin for this).

Now when slug is invalid raise a SlugError, that I create in core.exceptions and raise a HTTPException in RecipeController class in recipe_crud_routes.py file

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

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

Looks good, except for the language files; translations are handled through Crowdin. You should only add to/update the en-US.json file. Everything else shouldn't be modified here

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

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

I went ahead and finished this one up. I ended out removing the custom message and just stuck with the existing flow. The user can always check the logs if they're confused, but at least now it won't create an invalid recipe.

Thanks for your work on this!

@michael-genson michael-genson merged commit 107dfc3 into mealie-recipes:mealie-next Oct 24, 2025
15 checks passed
@fernandom06
Copy link
Contributor Author

I'm sorry the past few months have been really crazy. Please forgive my absence.

I think your solution is much cleaner and better suited for such a rare case.

Thank you so much for the opportunity to collaborate on this wonderful project.

@michael-genson
Copy link
Collaborator

No worries, appreciate your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] - recipe with name '---' can't be edit

2 participants