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

Ticket graph connection finding/handling patch #103

Closed
wants to merge 1 commit into
base: stable
from

Conversation

Projects
None yet
2 participants
@atus42
Copy link

atus42 commented Jun 24, 2014

The original way the ticket links were finded and handled has a flaw. If
a ticket was reached first in a 'longer' path/way than the shortes way
from the root ticket, it got a bigger CurrentDepth as it should be. Later
this ticket were skipped because it was 'already seen' but the
CurrentDepth were not checked.

To handle connections and CurrentDepth correctly, the whole algorithm
should be rewriten from depth-first to child-first search.

@atus42

This comment has been minimized.

Copy link

atus42 commented Jun 24, 2014

attached a before / after screenshot of the same graph.
You can see that ticket #50 has two "DependsOn" entry but one of them didn't drawed. It is because the loop in TicketLinks reach ticket #50 in this way: 40->48->50. The grap's main link type is 'MemberOf'. Because the link between 48 and 50 is DependsOn (not the main type) the links of ticket 50 is not processed. When the loop reaches ticket #50 directly from ticket #40, the ticket #50 is already on the '$args{'Seen'}' list, so it is skipped, so the link between #50 and #46.
graph_error
graph_ok

@atus42

This comment has been minimized.

Copy link

atus42 commented Aug 29, 2014

Can anyone review and merge this? Please.

@alexmv

This comment has been minimized.

Copy link
Contributor

alexmv commented Sep 3, 2014

Thanks for the patch; you're right that this should ideally be redesigned from a depth-first to a breadth-first search. Your patch is a fine stopgap -- however, the format commit message needs some minor tweaking. All of the lines should be < 75 characters, and the first line should be as succinct a summary as possible.

@atus42 atus42 changed the title This patch corrects an error in the graph. The original way the ticket l... Ticket graph connection finding/handling patch Sep 4, 2014

@atus42

This comment has been minimized.

Copy link

atus42 commented Sep 4, 2014

Changed the title and the first comment according to the suggestions you made.

Kadar Attila
Ticket graph connection finding/handling patch
The original way the ticket links were finded and handled has a flaw. If
a ticket was reached first in a 'longer' path/way than the shortes way
from the root ticket, it got a bigger CurrentDepth as it should be. Later
this ticket were skipped because it was 'already seen' but the
CurrentDepth were not checked.

To handle connections and CurrentDepth correctly, the whole algorithm
should be rewriten from depth-first to child-first search.

@atus42 atus42 force-pushed the atus42:patch--lib_RT_Graph_Tickets.pm branch from e0cef32 to 3303ec3 Sep 4, 2014

@atus42

This comment has been minimized.

Copy link

atus42 commented Sep 4, 2014

Also changed the git commit message.

@alexmv

This comment has been minimized.

Copy link
Contributor

alexmv commented Sep 4, 2014

I've reworded slightly and merged to 4.2-trunk. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment