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.

Sign up for GitHub

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

Jump to bottom

[Content Types] : Deleting field is throwing an error #20342

Closed
jcastro-dotcms opened this issue May 4, 2021 · 4 comments · Fixed by #20344 or #20379
Closed

[Content Types] : Deleting field is throwing an error #20342

jcastro-dotcms opened this issue May 4, 2021 · 4 comments · Fixed by #20344 or #20379
Assignees
@erickgonzalez @jcastro-dotcms
Labels
Changelog: Documented dotCMS : Content Management Merged QA : Approved QA : Passed Internal Release : 5.3.8.6 Included in LTS patch release 5.3.8.6 Type : Defect

Comments

@jcastro-dotcms
Copy link
Contributor

Reported via Support: 103972
Reproducible in dotCMS 5.3.6.3, and latest master

Deleting a field from a given Content Type is throwing an error related to a completely different field. The same thing happens when moving any field around. The error is related to a Relationship field, as it seems the code is having problems updating its properties.

To Reproduce
NOTE: Please reach out to Jose Castro for the specific data source, and the steps to reproduce. This issue is not reproducible with the Demo Starter:

Expected behavior
Users must be able to delete fields without problems.

Additional context
Went through the field delete logic with @freddyucv , and found a potential bug in our API.
@jcastro-dotcms jcastro-dotcms added this to the Maintenance Sprint milestone May 4, 2021
@jcastro-dotcms jcastro-dotcms self-assigned this May 4, 2021
jcastro-dotcms added a commit that referenced this issue May 4, 2021
@jcastro-dotcms
#20342 : As per discussion with Freddy R, we noticed that the API is …
6c8f1b7 
…trying to create relationships in any Relationship field even after simply deleting or moving other fields. The code in charge of moving and deleting fields is now setting the skip Relationship creation parameter to "true" so that this doesn't happen. Also, logging the error when the field layout is invalid, and adding Javadoc.
@jcastro-dotcms jcastro-dotcms linked a pull request May 4, 2021 that will close this issue
#20342 : As per discussion with Freddy R, we noticed that the API is … #20344
Merged
@jcastro-dotcms jcastro-dotcms added the LTS : Next Ticket that will be added to LTS label May 4, 2021
fmontes pushed a commit that referenced this issue May 7, 2021
@jcastro-dotcms
#20342 [Content Types] : Deleting field is throwing an error
f0dd36f 
As per discussion with Freddy R, we noticed that the API is trying to create relationships in any Relationship field even after simply deleting or moving other fields. The code in charge of moving and deleting fields is now setting the skip Relationship creation parameter to "true" so that this doesn't happen. Also, logging the error when the field layout is invalid, and adding Javadoc. (#20344)
@fmontes fmontes reopened this May 7, 2021
@fmontes fmontes added the Merged label May 7, 2021
@erickgonzalez
Copy link
Contributor

This is breaking the relationships, aren't being saved at all.

@erickgonzalez erickgonzalez self-assigned this May 11, 2021
@jcastro-dotcms jcastro-dotcms removed the LTS : Next Ticket that will be added to LTS label May 12, 2021
@jcastro-dotcms
Copy link
Contributor Author

Temporarily removing the LTS: Next label. As per the R&D Team, the original fix is causing problems in other places of the system, so the code changes need to be rolled back.

erickgonzalez added a commit that referenced this issue May 12, 2021
@erickgonzalez
#20342 check if the id is set
6f0101e
erickgonzalez added a commit that referenced this issue May 12, 2021
@erickgonzalez
#20342 tests
f7d2679
@erickgonzalez
Copy link
Contributor

PR for fixing problems that the original PR caused: #20379

To backport this to LTS you will need both PRs:
#20344 and #20379

@erickgonzalez erickgonzalez linked a pull request May 12, 2021 that will close this issue
Issue 20342 #20379
Merged
fmontes pushed a commit that referenced this issue May 18, 2021
@erickgonzalez
#20342 [Content Types] : Deleting field is throwing an error
762c7b7 
* #20342 check if the id is set

* #20342 tests
@fmontes fmontes added the Merged label May 18, 2021
@fmontes fmontes reopened this May 18, 2021
@jcastro-dotcms jcastro-dotcms added the LTS : Next Ticket that will be added to LTS label May 19, 2021
@bryanboza
Copy link
Member

Fixed, tested with a provided data set and now we are able to delete the field without problems. Tested on release-21.06 // Postgres // FF

@fmontes fmontes closed this as completed Jun 7, 2021
jcastro-dotcms added a commit that referenced this issue Aug 19, 2021
@jcastro-dotcms
#20342 : Back-porting code changes to dotCMS 5.3.8.6 LTS release.
a878d21
@jcastro-dotcms jcastro-dotcms added LTS: Released Release : 5.3.8.6 Included in LTS patch release 5.3.8.6 and removed LTS : Next Ticket that will be added to LTS labels Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@erickgonzalez erickgonzalez

@jcastro-dotcms jcastro-dotcms

Labels
Changelog: Documented dotCMS : Content Management Merged QA : Approved QA : Passed Internal Release : 5.3.8.6 Included in LTS patch release 5.3.8.6 Type : Defect
Projects
None yet
Milestone
No milestone
5 participants
@fmontes @bryanboza @rweiner @erickgonzalez @jcastro-dotcms