pkp-lib
pkp-lib copied to clipboard
In OJS, a reviewer cannot access his/her recommendations from a previous round.
As a reviewer reinvited to a new review round, I would like to be able the see the evaluation I provided in previous rounds.
In OJS, reviewers that are being invited to different review rounds on the same submission cannot access what was their previous recmmandations.
This improvement will offer the opportunity to reviewers to access their previous review reports and thus, better determine how the author made his/her changes to the manuscript.
The reviewer will be able to access his/her Previous round reports by clicking on a Round button on top of the reviewer process page (as shown below).
When clicking on a cycle button a new window open containing the following information:
- Recommandation
- Forms informations
- Review file sent
- Cycle dates
- Article version reviewed during preceding round
PRs:
Update (2024-04-02) pkp/ui-library#313 (jardakotesovec repository) pkp/pkp-lib#9526 pkp/ojs#4148
Initial: #9526
How to test?
- Make a submission and send it to the Review step.
- Invite one reviewer.
- Connect as reviewer and complete the review (fill the form and add a file).
- As editor, Request revision and select Revisions will be subject to a new round of peer reviews.
- As an author, submit revision.
- As editor, open a 2nd round of review.
- Invite the 1st round reviewer.
- Connect as the reviewer.
- On top of the review page, you should see the Round 1 bouton.
- In the new window, the reviewer can access its answers (forms and files).
PR: https://github.com/pkp/pkp-lib/pull/9526
@Vitaliy-1, could you review this? Thanks!
Thanks, @nibou230! I left a couple of comments.
Hi, @Vitaliy-1, just to let you know, I am a colleage of @nibou230, he will be on vacation for the next few weeks. Back in January. We will follow up when he comes back. Thanks!
@nibou230 Hi, thanks for the contribution, its certainly useful feature. Only drawback is that this is built on legacy stack and we aim to avoid adding new functionality on this old stack, because that means that we would need to migrate it to newer stack at some point anyway.
I would like to ask whether you would have some dev capacity & interest to build this feature on newer stack?
It would be creating frontend part in Vue.js (significant portion of OJS is already on Vue.js) and creating relevant API endpoints that the frontend would use.
If so, we would provide support and guidance how to do it. In particular frontend part: @jardakotesovec api: @defstat UI/UX: @Devika008
Let us know what you think
Thank you!
Jarda
Hi @jardakotesovec, if our planning let me some spare time, I will do my best to convert this feature on the newer Vue.js stack. Do you have any portion of the application (on Vue.js) that would be similar so I could take some inspiration from it?
Hello @Vitaliy-1, thanks for the comments, if they are still relevant, I will take a look at those once the refactor is done.
@nibou230 That would be great! Some of the things that we are currently using are quite new, therefore there is not enough examples, so I decided to create boilerplate to show how things should come together. New composition API from vue3 might look bit intimidating at first, but once you get use to it, its very expressive way to write logic for components.
Once the tests run, I will merge it to main (probably tomorrow morning), so you can rebase against main and start moving things over.
PR: ui-library, pkp-lib, ojs
Since I will be merging unfinished work to the main branch, its behind feature flag, to enable displaying of 'History buttons' adjust your config.inc.php to include:
[features]
enable_review_round_history = On
Now there are two big parts to tackle:
API
We already have many API endpoints to serve data - https://docs.pkp.sfu.ca/dev/api/ojs/3.4 . Unfortunately the data you are after are not exposed yet. So it will be necessary to introduce new ones that would cover needed metadata.
Can someone more familiar with that provide bit of guidance and suggest some good modern examples to follow? Pinging @asmecher @Vitaliy-1 @defstat
In boilerplate for demonstration purposes I am using the /submissions/{submissionId} endpoint to get submission metadata.
Styling
Thats probably last thing to tackle. For new UI we are using TailwindCSS - please checkout our documentation in storybook.
Pinging @Devika008 in case she would have capacity to mock what the UI could look like. If not - we will improvise :-).
Also note that one of the parts is listing of reviewer files - I will be building same Vue.js component hopefully in ~2weeks for new submissions listing. So thats something I would provide as well (but that handles just rendering, its still necessary to create API to fetch them).
Other notes
- Yes I noticed that buttons shows
Round 1
, and I am tempted to adjust the translations, but want to check with the team first, rather than fallback tov-html
which is way too permissive, just to be able pass the html entity. - Also feel free to explore the storybook that I linked above for additional technical details - I have not promoted it, as I am still working on documentation, but its lots of info already there. More officially it will be introduced in ~2/3weeks.
Wow, thank you so much @jardakotesovec!
I was quite lost trying to figure out how I could start this and how to link the component/page to the reviewer handler section. I'm more of a C# type of developer so it's far from my usual code. :-)
This boilerplate will help me a lot. I'll wait until tomorrow for this code and meanwhile I will look at how I should expose the required data.
Should I create a whole new API controller or should I add a method or two inside the current submission one?
@nibou230 Hi Nicolas, its now merged on main. If you have any further frontend related questions or feedback, let me know. And hopefully on API side someone will chip in shortly.
Hi @nibou230! Regarding the API, all the data that you are after seem to be contained into the Review process, so it could make sense to start with working on a separate API Controller and add your endpoints there. One example that you could investigate is the JATS API controller. If you have any questions regarding that don't hesitate to ask.
Depending on the URLs that the newly developed endpoints will listen to, there could be some specific changes that you need to make in order for those to work correctly, but I suggest to see those if there is such a need.
Please take into account possible differences between the OJS and OMP applications, so that each application (OJS or OMP) specific changes will be developed out of the PKP-LIB API Controller, and leave common changes (that cover both OMP and OJS) into the PKP-LIB API Controller.
You could check (for example):
- Submission API Controller for OJS
- Submission API for OMP No specific changes needed for OMP
- Submission API Controller for PKP-LIB - Common for all applications
Hi @defstat, thank you for the help.
I'll look at the Jats controller and make another one specific for the reviews.
I understand the point to split what should be common or specific to an application, but I don't know OMP at all. I will do my best and if you guys spot something that should not be inside PKP-LIB, tell me and I will make the appropriate change.
Hi @defstat, thank you for the help.
I'll look at the Jats controller and make another one specific for the reviews.
I understand the point to split what should be common or specific to an application, but I don't know OMP at all. I will do my best and if you guys spot something that should not be inside PKP-LIB, tell me and I will make the appropriate change.
Perfect, thanks. I would suggest not bothering with OMP specifics, at this stage. You have already much to consider for this change. :)
The application specifics can be handled during the reviewing stage, and after the feature is in a mature functioning state.
Hello,
I can do UI mock-ups for this keeping our updated design system in mind but will require till Tuesday the 23rd of January? Is that okay?
HI @Devika008, @defstat, @jardakotesovec Thanks very much for all your help on this!
Given the extensive refactoring and the fact that we need to adopt a very recent new technology, we propose to make a first MVP of this feature with:
- Standard UI using OJS components as is (as documented in StoryBook). The page will be structured using the section documented in the issue: Recommendations, Form content, Dates (see image below)
- For Author Manuscripts and Reviewer file, we will wait for Jarda works on submission to reuse the component he will have created.
- On first iteration, the form section will only display the default review forms, not custom forms
Do you agree with this scenario?
Hi guys @defstat @jardakotesovec,
I worked on the different PRs last week:
https://github.com/pkp/ui-library/pull/313 https://github.com/pkp/pkp-lib/pull/9526 https://github.com/pkp/ojs/pull/4148
It's not 100% done, the files are still missing, but you can already take a look if something needs to be addressed right away.
Thanks!
@nibou230 hi! Regarding your question on the reviewerId
and whether this is the way to go, or whether you need to add the reviewerId
as an input parameter to the request, I think you should consider the specific usecase here - would this issue result to a UI solution that will display "history" information only for the specific reviewer that is currently logged in, or will an editor for example have access to this information also?
If my understanding is correct, each reviewer will have access to his/her individual history. If this is the case then I think that getting the reviewerId
from the request
is sufficient.
@defstat OK, thank you! Yes, you are correct, the history side panel is visible only for the current logged in reviewer for now. I wanted to make sure I followed the standards for API development.
Hello @jardakotesovec @nibou230 @pilasou @defstat
Keeping the elements we currently have and the information architecture we are trying to establish through our revised designs (editorial decisions, email templates, submission dashboard, following is the UX/UI i propose for the issue.
Specifications:
- I have adapted the UX/UI of Revised Editorial Decisions. this includes the stepper mechanism instead of the tabs for a step by step journey. Tabs usually indicate that you can hop between them irrespective of any decision which doesnt support the mental modal here
- I have also introduced a white block for Previous Reviews. There could be edge cases where there are more than one review round and hence to make the design scalable I introduced the block.
- The information flow and design below the stepper remain constant to what is there currently
Specifications:
- On clicking the "Read Review" button, this panel will open just like it does in the summary section of new submission dashboard.
- The summary section of the dashboard was designed in the way that the grey area was used to support submission information or metadata and the white area to support any information that is important to made a decision. Hence, I've used the grey area here to higlight submission metadata and the general information and the white area to show the recommendations, comments, files and attachments
Let me know if this works for you or if you have any additional comments of feedback on this
@Devika008 Thats great update, thank you! Lets keep this focused on the history. Changing tabs to stepper make sense, but is out of the scope of this one. Maybe create new ticket with that idea and we will see whether we can execute it separately or later when the reviewer process will be revamped.
@nibou230 Based on the design from Devika, here is some guidance that should help you to get that going easily, just merged that to main:
- 2 Column layout for side modal available in storybook - you will find this example in SideModal.stories.js
- Listing files - added component to handle that.. its not pixel perfect per the design, but will refine that later. Here it is in storybook. Source code again in stories file.
Let me know if there is anything else you need. The section with Read Round buttons on top should be quite easy just style with Tailwind.
@jardakotesovec Thank you so much :D.
What ill do is that ill create a separate ticket for the stepper for this one. However, I feel it will be a good user experience if we retain the white block above the stepper and use it above the tabs to make it more scalable for the future
@jardakotesovec The storybook is really helpful! Thank you so much for the help, it makes my life better!
@Devika008 i'll try to follow the design :-) This is what I have for now, i'll continue the work in the following days.
Hi everyone, I'm mostly done with the development. You can see a preview of the review history here.
@jardakotesovec The URL used to download the file isn't working. Is this normal (still under development) at this point? It's trying to fetch "http://localhost:8000/index.php/partage/reviewer/submission/url".
This is amazing @nibou230. thank you so much
@nibou230 No idea, so far I just made the component that links to whatever url is passed to it. But to be honest not familiar with the endpoints for downloading files. I could look at it on Tuesday, but maybe someone more familiar will know the answer.
Hi everyone, I'm mostly done with the development. You can see a preview of the review history here.
@jardakotesovec The URL used to download the file isn't working. Is this normal (still under development) at this point? It's trying to fetch "http://localhost:8000/index.php/partage/reviewer/submission/url".
@nibou230 the URL that does not work is the one that you prepared for the needs of this development? If so, you may need to have some code written for the URL to work. We can see that together if this is the case.
Hi @jardakotesovec, I submitted a fix for the URL, you can see it here.
I also had to push another fix for the SubmissionFileAssignedReviewerAccessPolicy
.
@defstat No, the new component is using the URL fetched with the submissionFile repository. I found the error this morning, the File
component was trying to set the href
instead of :href
.
@nibou230 Thank you for fixing that. Can you please cherry-pick this commit to your ui-library branch to add this page to storybook? https://github.com/jardakotesovec/ui-library/commit/19200fb83992d5ae7559224a8fb0a25cec28dc14
Would like to try test workflow with Devika, so she can provide feedback in storybook when she can see mostly real thing.
Also you might have better API mocks while testing it - so feel free to replace them with your, especially if it tests wider range of metadata.
@jardakotesovec OK, I already cherry-picked the commit and I will look if I can improve the mocks.
@Vitaliy-1 @defstat @defstat @asmecher I think last opened problem here is whether it needs some adjustments for OPS/OMP. Do anyone of you know from top of your head what differences there might be for this particular feature? I am not familiar with ops/omp yet much.. will be learning more about it once extending new submission listing for ops&omp.
I would suggest, before a more specific review, to add the application specific (OJS) changes to OMP at least, and add PRs for both OMP and OPS for the tests to run. It will give a hint of whether those changes work as they supposed to on OMP and OPS.
@nibou230 if you have questions on how to prepare the PRs for OMP and OPS (and OJS) so that the tests can run on travis, don't hesitate to ask.
Thanks, @defstat, we can make the PRs for OMP and OPS, but we want to say that we have never work with those softwares. If all goes smoothly and all tests pass for both software, does this mean our PRs will be accepted?