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

#30277 API/Factory methods to insert in the unique_fields table #30332

Conversation

freddyDOTCMS
Copy link
Contributor

@freddyDOTCMS freddyDOTCMS commented Oct 11, 2024

Proposed Changes

@freddyDOTCMS freddyDOTCMS marked this pull request as draft October 11, 2024 17:10
@freddyDOTCMS freddyDOTCMS marked this pull request as ready for review October 11, 2024 22:03
@freddyDOTCMS freddyDOTCMS marked this pull request as draft October 14, 2024 20:18
auto-merge was automatically disabled October 14, 2024 20:18

Pull request was converted to draft

@freddyDOTCMS freddyDOTCMS marked this pull request as ready for review October 15, 2024 14:59
/**
* This API allow you to interact with the unique_fields table
*/
public interface UniqueFieldAPI {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need a separate API for unique fields? Why not using the existing Field API?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is ok but should be called UniqueAPI and be able to use with other things not only field
at the end it is key value stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcastro-dotcms it have too much logic bi itself so that is why I think a new API and Factory are better, also is a different table

@jdotcms Really it is just for field right now if in the future we need it to be used for more cases then we can rename it and change the method signature but for now I don't think we should do changes for something that maybe never happend

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 504efaa

…-unique_fields-table' of https://github.com/dotCMS/core into issue-30277-Create-API-Factory-methods-to-insert-in-the-unique_fields-table
/**
* This Helper allow you to interact with the unique_fields table
*/
public class UniqueFieldAPIHelper {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think helper is tied to Resources on dotCMS, for biz logic we used to use Util

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* @throws UniqueFieldValueDupliacatedException when the Value is duplicated
* @throws DotDataException when a DotDataException is throws
*/
public void insert(final UniqueFieldCriteria uniqueFieldCriteria, final String contentletId)
Copy link
Contributor

Choose a reason for hiding this comment

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

missing annotation here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*/
public void insert(final UniqueFieldCriteria uniqueFieldCriteria, final String contentletId)
throws UniqueFieldValueDupliacatedException, DotDataException {

Copy link
Contributor

Choose a reason for hiding this comment

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

may be add some logs for the support guys in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* @param supportingValues
*/
@Override
public void insert(final String key, final Map<String, Object> supportingValues) throws DotDataException {
Copy link
Contributor

Choose a reason for hiding this comment

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

the factory concept usually is next to an API, since you do not have any, and there are not any plans of covering more general cases, this is just a single line you may move straight to the Helper/API you have

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think is better kepp the Factory because I am going to include more SQL Statement here later

/**
* Throw if try to insert a duplicated register in unique_fiedls table
*/
public class UniqueFieldValueDupliacatedException extends Exception{
Copy link
Contributor

Choose a reason for hiding this comment

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

typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 0aa9ced

@freddyDOTCMS freddyDOTCMS marked this pull request as draft October 16, 2024 15:49
auto-merge was automatically disabled October 16, 2024 15:49

Pull request was converted to draft

@freddyDOTCMS freddyDOTCMS marked this pull request as ready for review October 25, 2024 22:17
@freddyDOTCMS freddyDOTCMS marked this pull request as draft October 25, 2024 22:22
freddyDOTCMS and others added 4 commits October 28, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants