Display correct number of labels by Anonymous Users in Admin Users table

[sbower213]

Resolves #791. Very minor SQL change to display the correct number of labels on the Anonymous Users table of the admin dashboard.

[misaugstad]

Nice find @sbower213! @Manaswis and I must both have assumed that there was only one entry in the audit_task_environment table per audit_task_id, but clearly that is not the case!

[jonfroehlich]

@misaugstad: can you expand a bit on this and explain why you would have thought this and what the implication was of thinking this (and how it may have affected other parts of the system)?

[misaugstad]

I guess we just wouldn't really expect ip address/browser/OS to change mid-street. The code that was written was done by @Manaswis and it was a ~35 line query (very complicated), and so something like that is just going to be missed at some point. I then based the query that I wrote off of that one, without looking at what else was in the audit_task_environment table (all we were getting from it was ip_address). So we were just not quite as thorough as we should have been there.

And as far as effect on the rest of the system, there are a couple other functions in that same file that do this same thing incorrectly. And those functions are only ever used on the admin dashboard (in that table that was pictured, and in the table on the overview page that shows number of anon users who visited the site today/yesterday).

I have not found anything relating to the tool that has been impacted by this, and the table where we originally found the issue seems to be the most important admin/analytics thing that was impacted.

[misaugstad]

@sbower213 could you just go through the rest of that file you were editing, and make the same fix in the other couple functions that make that mistake?

[jonfroehlich]

Thanks @misaugstad for the detailed feedback. Again, when we see very complicated queries like this, imo, its a symptom that we have a broken architecture and that a table or other schema modifications should be made to better support the query.

A second takeaway, which you also indicate, is that we just need to be more diligent about reviewing code and testing.

[misaugstad]

Looks good! And the numbers make more sense now :)