[WIP] Make checking optional for dependent encoding profiles#171
[WIP] Make checking optional for dependent encoding profiles#171zuntrax wants to merge 16 commits intocrs-tools:masterfrom
Conversation
7d93320 to
b5eacb9
Compare
| 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_ticket_id bigint) |
There was a problem hiding this comment.
@jjeising @pegro
Continuing discussion
This doesn't work, Ticket::expandRecording calls Ticket::queryPreviousState with state='preparing', differing from the current state of the ticket, which would usually be cutting.
I'd suggest using an additional parameter skip_dependent DEFAULT FALSE and adding a new function (maybe with a trigger as suggested) ticket_is_master to pass that in from outside and keep this function completely ticket agnostic.
There was a problem hiding this comment.
As I said before, if determining a next/previous ticket state depends on more as just the ticket type and state, I'm fine with changing all functions to just requiring a ticket_id and migrate all users accordingly.
There was a problem hiding this comment.
I'm fine with changing all functions to just requiring a ticket_id
The new issue here is: ticket_state_next is also used with a state different from the current state of the ticket (with param_ticket_id).
There was a problem hiding this comment.
If you want to query for all possible ticket states of the current ticket, I would then add a second optional parameter param_state as a state of reference which defaults to the current ticket state.
So you could call ticket_state_next(param_ticket_id) to get the next state in relation to the current state. And you could call ticket_state_next(param_ticket_id, param_ticket_state) to query for the next state the ticket would get advanced to, if the ticket would be in state param_ticket_state.
At least that's what I'd suggest.
There was a problem hiding this comment.
@zuntrax That's what we discussed previously, can you implement that?
|
|
||
| public static function isSkippable($state) { | ||
| return ($state === 'postencoded' || | ||
| $state === 'checking'); |
There was a problem hiding this comment.
Mh I don't really like hard-coding stuff like that here.
Do we usually allow this?
There was a problem hiding this comment.
I think this should be moved to tbl_state?
There was a problem hiding this comment.
I guess you mean tbl_ticket_state, but yes.
Another question: should this setting only be visible if any profiles with a dependency are assigned to the project?
There was a problem hiding this comment.
Another question: should this setting only be visible if any profiles with a dependency are assigned to the project?
I'm not sure if it is worth introducing additional complexity in the view. It's only two checkboxes at the moment, which have a default value.
|
|
||
| WHILE ret IS NOT NULL LOOP | ||
| SELECT * INTO next_state FROM ticket_state_next(param_project_id, param_ticket_type, ret); | ||
| SELECT * INTO next_state FROM ticket_state_next(param_ticket_id); |
There was a problem hiding this comment.
Note: eliminating ret from parameters will probably produce endless loops.
|
Superseded by #217. |
Closes #160
Resubmitted from #162