-
Notifications
You must be signed in to change notification settings - Fork 11
[WIP] Make checking optional for dependent encoding profiles #162
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
|
Implements #160. |
| SET ROLE TO postgres; | ||
|
|
||
| CREATE OR REPLACE FUNCTION ticket_state_next(param_project_id bigint, param_ticket_type enum_ticket_type, param_ticket_state enum_ticket_state) | ||
| CREATE OR REPLACE FUNCTION ticket_state_next(param_project_id bigint, param_ticket_type enum_ticket_type, param_ticket_state enum_ticket_state, param_ticket_id bigint default NULL) |
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.
Mh, not really convinced having that extra parameter is a good idea.
Before this change, the function was generic, independent of a specific ticket, hence all the parameters to answer the question at hand. But now it's for a single ticket, so you could go ahead and just have ticket_id as only parameter, since all the other information is stored in the database as well.
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.
Mh, not really convinced having that extra parameter is a good idea.
What do you suggest? Moving to ticket_id as only parameter or switching to encoding_profile_[version_]_id?
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.
Both would work. I don't have a strong opinion on this.
Are there any callers, where no ticket_id is available or hard to determine?
I'm all for simplifying, if possible.
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 should behave the same way as the old function when calling, without ticket_id, but we should check that again.
| ts2.ticket_state = param_ticket_state AND | ||
| (pts.skip_on_dependent = false OR | ||
| ( /* is master encoding ticket */ | ||
| SELECT ep.depends_on |
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.
Like with other properties before, for performance improvements a column "is_master" in tbl_ticket filled on INSERT e.g. using a trigger might be useful.
|
Superseded by #171. |
Migrated pull request from private repo, cleanup and thoroughly review necessary.