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

Members of DOAJ Team not showing up in completion reports #1379

Closed
dommitchell opened this issue Sep 11, 2017 · 16 comments
Closed

Members of DOAJ Team not showing up in completion reports #1379

dommitchell opened this issue Sep 11, 2017 · 16 comments

Comments

@dommitchell
Copy link
Contributor

'alejandra' and 'ilaria' both issing from completion by month and completion by year reports. How many others are missing??

alejandra's roles: api, admin, editor
ilaria's roles: api, admin, editor, associate_editor

Please fix and re-run August reports

@emanuil-tolev
Copy link
Contributor

emanuil-tolev commented Sep 11, 2017

What kind of reporting are you expecting for those users?

I'm not very familiar with the prov. reporting, but if they're admins then only accepting and rejecting applications seem to be counted. Whereas if you're editor then how many applications you set to "Ready" is counted.

It picks the best role that describes you, with "admin" trumping all others. Is it possible these 2 users didn't accept/reject any applications in August?

I suppose it might be helpful to have them at 0 in that case. I would've thought users missing if they haven't taken any actions was tested when the reports were originally developed. No idea if the reports were initially aimed at the "see if the user has done nothing in this period" use case.

Tech thing: https://github.com/DOAJ/doaj/blob/develop/portality/tasks/reporting.py#L188

@emanuil-tolev
Copy link
Contributor

cc @richard-jones a +1 for the decision log ;)

@richard-jones
Copy link
Contributor

Yes, it would be good to remember why we made this decision about the reporting. As Emanuil says, reports for a given user are done based on their most senior role. Since both of these users are admins, only accept and reject events are reported on, regarding status changes. It should also report on "edit" events for these users though.

@dommitchell - what outputs are you expecting for these users? They would only appear in the reports if they did actually do something, as the report is based on the provenance records in the system. If a user doesn't have a provenance record, they won't be in the report.

As the provenance records are kept, we can always adjust the parameters/behaviours of the reporting scripts to count things differently.

@dommitchell dommitchell changed the title Two members of DOAJ Team not showing up in completion reports Members of DOAJ Team not showing up in completion reports Sep 11, 2017
@dommitchell
Copy link
Contributor Author

dommitchell commented Sep 11, 2017

Number applications completed per month.xlsx

It picks the best role that describes you, with "admin" trumping all others. Is it possible these 2 users didn't accept/reject any applications in August?

First off, the reports build month on month so this isn't just August. If you look at the attached, they are missing entirely.
Secondly, it's not just Alejandra and Ilaria but all of the Ambassadors are missing too. I just noticed that.
Thirdly, I know from #1044 (comment) that for a record to be counted in that report, the statusses must have been accepted, ready, completed, rejected. These 4 statuses cover everything that Alejandra and Ilaria and the Ambassadors do.

See the report sent to me, attached here.

Thanks for your help!

@richard-jones
Copy link
Contributor

Ok, we figured this out! There's a bug where if you have the role "api" as your first role, it won't count anything you did. We'll get a fix in for this shortly, and can re-run all the reports retrospectively.

@Steven-Eardley
Copy link
Contributor

Thanks to @richard-jones for the fix. I've re-run the reports using the following command on the background server:

export DOAJENV=production ; /home/cloo/repl/production/doaj/bin/python /home/cloo/repl/production/doaj/src/doaj/portality/scripts/reports.py -o /home/cloo/reports/2017-09-01_corrected -t 2017-09-01T00:00:00Z -e

So you should see the reports in your emails. They're also here for your convenience.

2017-09-01_corrected.zip

@ClaraDOAJ
Copy link
Contributor

Hello @Steven-Eardley Thanks for that!
The reports now show our editors, but not the ambassadors:

kamel
pascalsou
ivonne
inasmith
solomon
cenyu *this one is on the report but the numbers indicate 0 (not sure it is correct)
xinbi *curiously this one is on the report
yanhong
sridhar
leena
vrushali
mahmoud

Could you please check and re run?

@greboun
Copy link
Contributor

greboun commented Sep 13, 2017 via email

@Steven-Eardley
Copy link
Contributor

Steven-Eardley commented Sep 13, 2017

Referring again to Emanuil's comment above, the code we use to determine whether to include something in the report is this:

def _is_countable(self, prov, role):
    countable = False

    if role == "admin" and (prov.action == "status:accepted" or prov.action == "status:rejected"):
        countable = True
    elif role == "editor" and prov.action == "status:ready":
        countable = True
    elif role == "associate_editor" and prov.action == "status:completed":
        countable = True

    return countable

Since we look for a user's 'best role', if a user has role editor we will only include them in the report for actions where they set an application status to ready. For the ambassadors above, I see they all have the roles [api, editor, associate_editor] so they are considered editors.

Tom was right when he said that's why Cenyu shows 0 for this month, she's an admin and only has 2 rejections, back in February (which you can see on the report).

I've verified this by searching the live index using the following query:

{
    "query": {
    	"filtered": {
    		"filter": {
                "bool": {
                    "must": [
                        {"term" : {"action" : "status:ready"}}
                    ]
                }
            },
            "query": {
                "term" : {"user": "ivonne"}
		    }
	    }
    },
    "sort": [{"last_updated" : "desc"}]
}

and I get no results because she has not set anything to ready.

If I pretend Ivonne is an associate editor, and check for status:completed, I get 373 total records. These would appear in the report if Ivonne wasn't an editor, therefore having associate_editor as best role.

So, the reports are working as originally specified. It's up to the DOAJ team whether:

  • Ambassadors should be editors, thereby only counting their ready changes
  • The reports only show your actions at your highest role

If you'd like to change the reports, we can look at that under tnm.

@Steven-Eardley
Copy link
Contributor

Additionally, Xinbi is included in the report because they have in fact been setting applications to ready, so their actions are counted under the editor role.

@greboun
Copy link
Contributor

greboun commented Sep 13, 2017 via email

@greboun
Copy link
Contributor

greboun commented Sep 14, 2017 via email

@dommitchell
Copy link
Contributor Author

Steve, what's your thoughts on this one please?

@richard-jones
Copy link
Contributor

It looks like this might just be a modification in requirements as to how we count things. We should probably just carefully re-define what the report should be showing. Right now the "completed" report shows "completion" actions, dependent on user class, and assumes that users do not have more than one class (e.g. that administrators are not associate_editors).

It's easy enough to modify these rules, and we have all the data from the provenance system, so we can re-run the reports with new rules to see the new answers.

Some options occur to me:

  1. Count all "completion" events, irrespective of user class. This means that the number next to a user's name will be a combination of items set to ready, completed, rejected and accepted. You wouldn't be able to pull those apart from the data in the report, and would you want to?

  2. Have a separate report for each user class, and count a user in every report which is relevant to them. So, a user may be in the "admin" sheet for 200 completions, and in the "editor" sheet for 20 completions, and in the "associate editor" sheet for 2 completions.

  3. Define a new role called "ambassador", and add a new rule to count completions by ambassadors as you need.

We could discuss this at our meeting tomorrow.

@Steven-Eardley
Copy link
Contributor

Alternatively, if you remove the editor roles from the ambassadors, their completed status changes will be counted without any changes to the reports and I could just re-run them.

I'd recommend modifying the reports, though - we could get a more complete picture of what is happening if we change the data collection restrictions.

@dommitchell
Copy link
Contributor Author

@Steven-Eardley @richard-jones thanks for your analysis.

  1. Count all "completion" events, irrespective of user class. This means that the number next to a user's name will be a combination of items set to ready, completed, rejected and accepted. You wouldn't be able to pull those apart from the data in the report, and would you want to?

No, we wouldn't need to at this stage.

  1. Have a separate report for each user class, and count a user in every report which is relevant to them. So, a user may be in the "admin" sheet for 200 completions, and in the "editor" sheet for 20 completions, and in the "associate editor" sheet for 2 completions.

I actually think it would be good to have both options (1) and (2) so that we get the total activity and then a breakdown, should we need it.

But let's discuss this on our call today.

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

No branches or pull requests

6 participants