pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
299 stars 444 forks source link

Integrate statistics Custom Report Generator with article statistics UI #7318

Closed NateWr closed 1 year ago

NateWr commented 3 years ago

Describe the problem you would like to solve The custom report generator (Statistics > Reports > Generate Custom Report) duplicates some of the filtering and sorting options in the article stats UI (Statistics > Articles). The custom report UI is confusing, and depending on which report template is used, offers advanced options that don't apply to the report.

Describe the solution you'd like Generating a custom report should be integrated with the article stats UI, so that when a journal manager is viewing the article statistics they generate a report from the date and section filters they have already selected.

The journal manager can click to generate a particular report, and then get a few additional options to configure the output they receive. Because the UI already includes tools to filter by date and daily/monthly, the options to configure a custom report would be much simpler. The JM would only need to select the columns they want to include.

reports

Who is asking for this feature? Tell us what kind of users are requesting this feature. Example: Journal Editors, Journal Administrators, Technical Support, Authors, Reviewers, etc.

Additional information An inventory exercise was conducted to understand all of the requirements for statistics reports. That can be found at: https://pkp.notion.site/d3078b32275d4b8a98fe65d5b77d125e?v=188ebb82537c4b8997ddb82f86193477

TO-DOs:

NateWr commented 1 year ago

@bozana the PRs at https://github.com/pkp/pkp-lib/issues/7318#issuecomment-1275103834 look good. However, I noticed a couple of things:

  1. The pkp-lib branch is named 7318 but the ui-library branch is named 7318-issues. Your submodule commit message is pkp/pkp-lib#7318 pkp-lib submodule update ##bozana/7318##. So the tests will check out the right branch in pkp-lib but not ui-library. The tests still pass, but this probably just indicates that there are no tests related to the changes in ui-library.
  2. When I downloaded a geographic stats report, it looks like the issue row in the report attributes has an extra first column:
"Date Range","2021-10-27 to 2022-10-26"
"Sections","All Sections"
,"Issues","All Issues"

City,Region,Country,Total,Unique
Tijuana,"Baja California",Mexico,3,0
unknown,unknown,"United States",1,0
Santiago,"Región Metropolitana de Santiago",Chile,1,0
Reutlingen,Baden-Württemberg,Germany,1,0
Laurent,FR-D,France,1,0
Mexico,"Ciudad de México",Mexico,1,0
Guadalajara,Jalisco,Mexico,1,0
Houston,Texas,"United States",1,0

Otherwise, it all looks good and I'm happy for this to be merged once you're ready.

bozana commented 1 year ago

Hi @NateWr, thanks a lot!!! Hmmm... I did not know that all submodules have to be in the same named branch i.e. all need ##bozana/branch## -- I thought only pkp-lib... I will change the branch for the pkp-lib PR -- I noticed that recently but did not to make the new PR because of the comments... But now, directly before the merge, I can do that... Ah, I know where is the problem with the ',' -- I wrongly treated the array of filter params so that they are imploded with ','... Will fix that! Thanks a LOT!!!

bozana commented 1 year ago

PRs for Stats Context Page: ui-library: https://github.com/pkp/ui-library/pull/226 pkp-lib: https://github.com/pkp/pkp-lib/pull/8383 ojs: https://github.com/pkp/ojs/pull/3593 omp: https://github.com/pkp/omp/pull/1234 ops: https://github.com/pkp/ops/pull/381

bozana commented 1 year ago

@NateWr, I think/hope these are the last PRs for this issue, for Stats Context Page. Could you take a look? I only considered the stats for the given context (journal, press, server) i.e. get() and getTimeline() and not the getMany() and getManyTimeline() that only admin has access to. I am even not sure if it is worth to consider that UI to for the admins... ? I renamed a few locale keys -- shortly before the merge I will synchronize it with Alec... Thanks a lot!

NateWr commented 1 year ago

Thanks @bozana! @ewhanson would you be able to do the code review on this? It should follow the same patterns set out in the articles and issue stats pages.

ewhanson commented 1 year ago

Hey @bozana and @NateWr. I've completed my code review, and it looks great. I just had a few comments I left in line. Let me know if you have any questions.

bozana commented 1 year ago

Thanks a lot @ewhanson! I fixed the getParams() method in StatsContextPage.vue. I've rebased and prepared everything... So if no further comments, questions, objections... I would merge once the tests pass...

ewhanson commented 1 year ago

Hey @bozana, that sounds good to me!

bozana commented 1 year ago

PRs for renaming the locale keys: https://github.com/pkp/pkp-lib/pull/8416 https://github.com/pkp/ojs/pull/3617 https://github.com/pkp/omp/pull/1247 (only submodule update) https://github.com/pkp/ops/pull/390 (only submodule update)