-
Notifications
You must be signed in to change notification settings - Fork 7
Throw exception when lsid is not found in table selector list #3227
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
Conversation
| } | ||
| else | ||
| { | ||
| throw new NotFoundException("LSID value not found in table - " + table.getName()); |
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.
NotFoundException is useful when the client set us a request for something that doesn't exist. We don't log it as an error because it was a bogus request.
IllegalStateException is a better fit here, as this represents something that shouldn't happen. It'll get logged and be more likely to be noticed with test failures.
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.
makes sense. changed.
labkey-jeckels
left a comment
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 know you're still tracking down test failures but the fact that this change exposed those problems is a good confirmation of the approach.
| import org.labkey.api.security.permissions.Permission; | ||
| import org.labkey.api.security.permissions.UpdatePermission; | ||
| import org.labkey.api.util.Pair; | ||
| import org.labkey.api.view.NotFoundException; |
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 can be trimmed now.
Rationale
EHR billing tables have extensible columns and when those table rows are updated, the existing values for extensible columns get lost when lsid is not found in table selector.
Related Pull Requests