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

Make sure that creating a new Organization accepts the "slug" field #105

Closed
wants to merge 2 commits into from

Conversation

jhodapp
Copy link
Member

@jhodapp jhodapp commented Mar 3, 2025

Description

This PR ensures that when creating a new Organization instance, it accepts the slug field and this value is unique across all Organizations.

GitHub Issue: None

Changes

  • Accept the slug field as non-optional
  • Ensure slug's value is unique across all Organizations

Testing Strategy

  1. Create a new organization with RAPIdoc

Concerns

None

@jhodapp jhodapp added the bug fix Contains a fix to a known bug label Mar 3, 2025
@jhodapp jhodapp added this to the 1.0-beta1 milestone Mar 3, 2025
@jhodapp jhodapp requested a review from calebbourg March 3, 2025 01:19
@jhodapp jhodapp self-assigned this Mar 3, 2025
Copy link
Collaborator

@calebbourg calebbourg left a comment

Choose a reason for hiding this comment

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

I'd like us to generate slug values in the BE

@@ -15,7 +15,7 @@ pub struct Model {
#[sea_orm(unique)]
pub name: String,
pub logo: Option<String>,
#[serde(skip_deserializing)]
#[sea_orm(unique)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

First of all, awesome find on the unique macro! Without looking at the docs, I wonder if we can express something like unique in the scope of {other associated record}. We'll need something like that for coaching relationships.

As far as generating the slugs on the FE, I have a fairly strong opinion that this should be a BE operation. The way I had envisioned it would be that a mechanism would exist to derive a slug by translating the name field automatically at the time of creation. The reasons for having it done of the BE:

  • The slug is designed for programmability due to its strict standardized requirements, and its immutability. I'd prefer to use BE technology to handle the rules involved and the string manipulation.
    -The generation of a slug cannot be susceptible to savy user's modifying client code as we will be depending on characteristics of the slug (Ex. hyphenated, lowercase, etc.) in our downstream code.

We, of course, will add validations on the format of a slug on the BE but I'd like to keep this logic and responsibility out of the FE if possible to keep things simple and thin.

Let me know what you think about that.

@jhodapp
Copy link
Member Author

jhodapp commented Mar 3, 2025

Oh I see. Something still needs to change then because creating a new Organization without providing a value for 'slug' was failing to insert claiming I still needed to set a value.

@calebbourg
Copy link
Collaborator

@jhodapp yeah that's right. We need to add a step in the creation process where the input name is translated into a slug in some deterministic way.

If this is a blocker for you at the moment (assuming you need it to seed orgs with the tool you built)I'm happy to approve this PR now and come back and move the slug creation to the BE later.

@jhodapp
Copy link
Member Author

jhodapp commented Mar 3, 2025

@jhodapp yeah that's right. We need to add a step in the creation process where the input name is translated into a slug in some deterministic way.

If this is a blocker for you at the moment (assuming you need it to seed orgs with the tool you built)I'm happy to approve this PR now and come back and move the slug creation to the BE later.

Thanks for that flexibility Caleb, but I do think we should do it right, right now. Or at the very least make it so that the frontend can create a new Organization without providing a slug and still have it insert into the DB successfully.

I'm happy to take a crack at this so you can keep moving forward on your feature work.

@jhodapp
Copy link
Member Author

jhodapp commented Mar 5, 2025

Superseded by #107

@jhodapp jhodapp closed this Mar 5, 2025
@github-project-automation github-project-automation bot moved this from Review to ✅ Done in Refactor Coaching Platform Mar 5, 2025
@calebbourg calebbourg deleted the fix_create_organization branch April 4, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Contains a fix to a known bug
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants