Skip to content
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

[Conflict Resolver] Enabling Active Candidate and Active Session Check in Query #3792

Merged
merged 4 commits into from
Jul 12, 2018

Conversation

sruthymathew123
Copy link
Contributor

@sruthymathew123 sruthymathew123 commented Jul 9, 2018

Redmine 14872

Conflict Resolver is pulling results of Inactive Candidates and Inactive Sessions. This leads to one of the reason for IBIS where our conflicts count in dashboard doesn't match with the count in conflict resolver.

@sruthymathew123 sruthymathew123 added the Category: Bug PR or issue that aims to report or fix a bug label Jul 9, 2018
@sruthymathew123 sruthymathew123 changed the title [Conflict Resolver] Enabling Active Candidate Checking [Conflict Resolver] Enabling Active Candidate Check in Query Jul 9, 2018
@@ -236,7 +236,7 @@ class Conflict_Resolver extends \NDB_Menu_Filter_Form
LEFT JOIN session ON (flag.SessionID=session.ID)
LEFT JOIN candidate ON (candidate.CandID=session.CandID)
LEFT JOIN Project ON (candidate.ProjectID=Project.ProjectID )
WHERE 1=1";
WHERE 1=1 AND candidate.Active ='Y'";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can take out the 1=1. That's just there to ensure that if filters add "AND" clauses it doesn't become an SQL syntax error but is unnecessary if there's already a WHERE clause in the query.

@@ -84,7 +84,7 @@ class Resolved_Conflicts extends \NDB_Menu_Filter
LEFT JOIN session ON (flag.SessionID=session.ID)
LEFT JOIN candidate ON (candidate.CandID=session.CandID)
LEFT JOIN Project ON (candidate.ProjectID=Project.ProjectID )
WHERE 1=1";
WHERE 1=1 AND candidate.Active ='Y'";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@sruthymathew123
Copy link
Contributor Author

@driusan Done the requested changes.

Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested

@xlecours
Copy link
Contributor

@sruthymathew123 The dashboard also filter on session.Active = 'Y'
should this query do it as well?

@sruthymathew123
Copy link
Contributor Author

@xlecours Good question. I think we have to consider that also.

@kongtiaowang
Copy link
Contributor

@sruthymathew123
You have changed the query conditon,but the testing data doesn't update.
So put "$this->markTestSkipped('rewrite this part later')" into the failed cases.

@sruthymathew123 sruthymathew123 changed the title [Conflict Resolver] Enabling Active Candidate Check in Query [Conflict Resolver] Enabling Active Candidate and Active Session Check in Query Jul 10, 2018
@driusan driusan merged commit 330634e into aces:20.0-release Jul 12, 2018
@ridz1208 ridz1208 added this to the 20.0.0 milestone Jul 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug PR or issue that aims to report or fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants