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

[20.0][Dashboard] Fix discrepancy in data entry conflicts #3775

Merged

Conversation

davidblader
Copy link
Contributor

The count in dashboard for conflicts is different from the count shown in the conflict resolver module
This makes the query responsible for retrieving the count closer to what's used in the conflict resolver module

@davidblader davidblader added the Category: Bug PR or issue that aims to report or fix a bug label Jun 28, 2018
@davidblader davidblader added this to the 20.0.0 milestone Jun 28, 2018
Copy link
Contributor

@PapillonMcGill PapillonMcGill left a comment

Choose a reason for hiding this comment

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

approval pending Travis agreement

@driusan
Copy link
Collaborator

driusan commented Jul 3, 2018

@me-evans Would you be able to test that these numbers are more reliable than the old query?

@me-evans
Copy link

me-evans commented Jul 3, 2018

@driusan - sure, no problem. Just need PR to be merged to an IBIS dev to be able to compare numbers with our prod.

@sruthymathew123 - Would you mind merging this PR to your VM (if dev up to date so that total conflicts in the module matches total on production)? Please let me know if yes, then I can access and complete testing requested by Dave.

Thanks!

@PapillonMcGill
Copy link
Contributor

@driusan I check the queries and they do use the same tables and joins. The WHERE clause are build differently but the result should be pretty close.

@davidblader
Copy link
Contributor Author

image
this is apparently passing travis but without updating on GH itself

@PapillonMcGill
Copy link
Contributor

restarted Travis. migth have been a bug during data transmission from Travis to GH.

@ZainVirani ZainVirani added the Passed manual tests PR has been successfully tested by at least one peer label Jul 9, 2018
@me-evans
Copy link

me-evans commented Jul 9, 2018

@driusan @davidblader - tested PR on Sruthy's dev with IBIS numbers, and there is no difference in the conflict totals displayed on the dashboard, with or without PR. What was supposed to change in terms of the criteria considered for the dashboard totals after merging the PR?
I've identified what the discrepancies are between dashboard & module, though:

  1. The total in the module is incorrect for one site, not the dashboard total. The module counts/displays two conflicts for a candidate whose Active flag has been set to No, and whose data should no longer be anywhere on the front end. The module should be changed LORIS-wide to exclude these candidates - made redmine ticket #14872 for this.

  2. The total conflicts displayed on the dashboard when user has "Across all sites" permission automatically excludes conflicts for DCC candidates, which do appear in the module. I'm guessing this was by design as it makes sense to me that users/coordinators would not want to see DCC conflicts included in the total for the project in this "at a glance" section.

(NOTE: The only thing that's a bit strange is that a user that has DCC as their (or one of their) site affiliation(s) will only see DCC conflicts in the dashboard total if they do not have the "across all sites" permission. I don't think this needs to be changed because DCC staff who use the dashboard as a reference (ex. coordinator) would have the "across all sites" permission and wouldn't want to see DCC conflicts included in the total. However, it could potentially cause some confusion for a DCC user testing permissions/multi-site that isn't aware of this exclusion and flags the discrepancy with the module totals. Maybe adding a small note in the My Tasks section when the permission is selected to indicate DCC not included could address this?)

@driusan driusan merged commit 1e06f22 into aces:20.0-release Jul 9, 2018
@driusan
Copy link
Collaborator

driusan commented Jul 9, 2018

Thanks @me-evans. It looks like @sruthymathew123 already sent #3792 for the first point, and I guess there's nothing to do for the second point. Maybe we should document it in the test plan?

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 Passed manual tests PR has been successfully tested by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants