-
Notifications
You must be signed in to change notification settings - Fork 291
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
[OPIK-735] Online Scoring: null projectId error when creating a rule #1011
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep a good encapsulation of the POJOs. Although not a blocker, you can send a follow up PR.
@NotNull UUID projectId; | ||
@JsonView({View.Public.class}) | ||
@Schema(accessMode = Schema.AccessMode.READ_ONLY) | ||
UUID projectId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is marked with @Data
so it has setters and getters. However, all these fields should be marked as private
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that this is meant to be extendable, so ok to have default visibility or protected for extension outside the package. You can still opt to narrow down visibility to private if you prefer.
Details
Using the endpoint, if the projectId is null in the payload, but ok in the path parameter, the rule creationg is aborted due to a 'projectId cant be null'.
During the triage I can see the service was using the projectId from the payload (which is optional and service should not use), while the correct would be use from the path param.
Issues
Resolves #
Testing
Documentation