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

As an API user, I shouldn't be able to update isCustom on an object or a field metadata #6581

Open
Weiko opened this issue Aug 8, 2024 · 24 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@Weiko
Copy link
Member

Weiko commented Aug 8, 2024

Bug Description

Many part of the code assume that isCustom:true is from the user input and should be ignore whereas isCustom:false is standard and should be kept in sync with the codebase entities.
However, even if an object or a field is automatically created as isCustom: true via the API, we can also update those field/objects and change that value which will break a few parts of the app including metadata sync scripts.

mutation UpdateOneField($input: UpdateOneFieldMetadataInput!) {
  updateOneField(input: $input) {
    id
    isCustom
  }
}

This input should not be possible, isCustom should not be available.

{
  "input": {
    "id": "1a3df789-8f19-4962-be35-34f59001b966"
  "update": {
    "isCustom": false
  }
}

Technical inputs

Do not expose isCustom in the graphql input field type

@Faisal-imtiyaz123
Copy link
Contributor

@Weiko I would like to work on it.

@prateekj117
Copy link
Contributor

@Weiko @charlesBochet @FelixMalfait Can I take this up? Seems quite straightforward. I see we need to remove the isCustom from the twenty-server from class UpdateFieldInput. And also remove it from UpdateFieldInput defined on frontend. I don't see any tests breaking by this change from code, but if would be there, will fix it too.

@FelixMalfait
Copy link
Member

@prateekj117 thanks for providing a plan!
I think that could work. We want to do it both at the field level and the object level. We need to make sure custom objects are always created with isCustom=true then, and that standard objects are always created with isCustom=false, even if it's not set by the api, which means it needs to be set at a different level in code.
Currently the default value for isCustom is false, it might have been slightly easier if the value had been true, but not sure. TBC if a change is needed.
Assigning you if you're still up for it?

@Faisal-imtiyaz123 thanks again! don't hesitate to give directions when you're looking to take something, that way it's helpful to validate it together.

@charlesBochet charlesBochet added the good first issue Good for newcomers label Aug 14, 2024
@Mohamedkaif10
Copy link

@Weiko @FelixMalfait I would like to work on this issue.

@FelixMalfait
Copy link
Member

Maybe @Faisal-imtiyaz123 wants to grab it first? Since @prateekj117 didn't reply.
Otherwise @Mohamedkaif10 go ahead in case @Faisal-imtiyaz123 doesn't reply in 24hrs.
Thanks to all of you!

@Faisal-imtiyaz123
Copy link
Contributor

@FelixMalfait You may assign it to @Mohamedkaif10 .

@FelixMalfait
Copy link
Member

Done! @charlesBochet marked this as a good first issue and it's doable, but I wouldn't say it's very easy as it requires at least a good understanding of what standard VS custom objects are and how they behave differently

@Mohamedkaif10
Copy link

Yeah now , i removed isCustom from the twenty-frontend(graphql.ts file) and also from mutation. suggest if other changes needed ..@FelixMalfait

@Bonapara
Copy link
Member

Bonapara commented Oct 1, 2024

/oss.gg 150

Copy link

oss-gg bot commented Oct 1, 2024

Thanks for opening an issue! It's live on oss.gg!

@anand1564
Copy link

/assign

Copy link

oss-gg bot commented Oct 7, 2024

Assigned to @anand1564! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

Copy link

oss-gg bot commented Oct 10, 2024

@anand1564, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@ManpreetKhinda
Copy link

/assign

Copy link

oss-gg bot commented Oct 10, 2024

This issue is already assigned to another person. Please find more issues here.

Copy link

oss-gg bot commented Oct 12, 2024

@anand1564, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@anand1564 anand1564 removed their assignment Oct 12, 2024
Copy link

oss-gg bot commented Oct 12, 2024

Assigned to @YashMehetre! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

Copy link

oss-gg bot commented Oct 14, 2024

@YashMehetre, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

Copy link

oss-gg bot commented Oct 16, 2024

@YashMehetre, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@PreethiMaddikunta
Copy link

/assign

Copy link

oss-gg bot commented Oct 17, 2024

Assigned to @PreethiMaddikunta! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

@PreethiMaddikunta PreethiMaddikunta removed their assignment Oct 18, 2024
@m3tal10
Copy link

m3tal10 commented Oct 18, 2024

/assign

@oss-gg oss-gg bot assigned m3tal10 Oct 18, 2024
Copy link

oss-gg bot commented Oct 18, 2024

Assigned to @m3tal10! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

Copy link

oss-gg bot commented Oct 20, 2024

@PreethiMaddikunta, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: 🆕 New
Development

No branches or pull requests