pkp/pkp-lib#2433 editor recommendation #2749

bozana · 2017-08-28T15:41:00Z

NateWr

👍

NateWr · 2017-08-29T09:16:59Z

classes/controllers/modals/editorDecision/PKPEditorDecisionHandler.inc.php

+	 * Show the editor recommendation form
+	 * @param $args array
+	 * @param $request PKPRequest
+	 * @return string Serialized JSON object


I think the @return is actually a JSONMessage object. It doesn't get converted to a string until later.

NateWr · 2017-08-29T09:17:19Z

classes/controllers/modals/editorDecision/PKPEditorDecisionHandler.inc.php

+	 * Show the editor recommendation form
+	 * @param $args array
+	 * @param $request PKPRequest
+	 * @return string Serialized JSON object


Same @return question as above.

NateWr · 2017-08-29T09:41:45Z

controllers/modals/editorDecision/form/RecommendationForm.inc.php

+		foreach ($editorsStageAssignments as $editorsStageAssignment) {
+			if (!$editorsStageAssignment->getRecommendOnly()) {
+				$editorFullName = $userDao->getUserFullName($editorsStageAssignment->getUserId());
+				$editorsStr .= ($i == 0) ? $editorFullName : ', ' . $editorFullName;


I was going to say that the separator , should be translatable like the authors string, but it looks like the authors string separator isn't translatable either. So I guess it's alright as-is. We don't seem to get complaints. :)

NateWr · 2017-08-29T09:42:39Z

controllers/modals/editorDecision/form/RecommendationForm.inc.php

+		$editDecisionDao = DAORegistry::getDAO('EditDecisionDAO');
+		$editorRecommendations = $editDecisionDao->getEditorDecisions($submission->getId(), $this->getStageId(), null, $user->getId());
+
+


Extra empty line.

NateWr · 2017-08-29T09:48:04Z

controllers/modals/editorDecision/form/RecommendationForm.inc.php

+			if (!$editorsStageAssignment->getRecommendOnly()) {
+				$editor = $userDao->getById($editorsStageAssignment->getUserId());
+				$editorFullName = $editor->getFullName();
+				$editorsStr .= ($i == 0) ? $editorFullName : ', ' . $editorFullName;


Might be good to extract the $editorsStr compilation into a method on the form, so it's easier to keep the presentation consistent.

I think I was wrong here -- here I do not need the editorStr -- so I will leave the loops as they are... ok?

NateWr · 2017-08-29T10:10:29Z

locale/en_US/manager.xml

@@ -429,6 +429,7 @@
 	<message key="settings.roles.roleOptions">Role Options</message>
 	<message key="settings.roles.showTitles">Show role title in contributor list</message>
 	<message key="settings.roles.permitSelfRegistration">Allow user self-registration</message>
+	<message key="settings.roles.recommendOnly">This role can not make a review decission, only a recommendation.</message>


This role is only allowed to recommend a review decision and will require an authorised editor to record a final decision.

NateWr · 2017-08-29T10:11:06Z

locale/en_US/submission.xml

@@ -520,6 +520,9 @@
 	<message key="editor.submission.roundStatus.reviewsReady">New reviews have been submitted.</message>
 	<message key="editor.submission.roundStatus.reviewsCompleted">All reviews are in and a decision is needed.</message>
 	<message key="editor.submission.roundStatus.reviewOverdue">A review is overdue.</message>
+	<message key="editor.submission.roundStatus.pendingRecommendations">Awaiting recommendations from editors.</message>
+	<message key="editor.submission.roundStatus.recommendationsReady">New recommendations have been submitted.</message>


New editorial recommendations have been submitted.

NateWr · 2017-08-29T10:11:26Z

locale/en_US/submission.xml

@@ -547,6 +550,11 @@
 	<message key="editor.submission.decision.approveProofs">Approve Proofs</message>
 	<message key="editor.submission.decision.disapproveProofsDescription">This file proof will no longer be publicly available for download or purchase. Do you want to disapprove it?</message>
 	<message key="editor.submission.decision.noDecisionsAvailable">Assign an editor to enable the editorial decisions for this stage.</message>
+	<!--  editor recommendations -->
+	<message key="editor.submission.recommend">Recommend</message>


Make Recommendation

NateWr · 2017-08-29T10:12:13Z

locale/en_US/submission.xml

+	<!--  editor recommendations -->
+	<message key="editor.submission.recommend">Recommend</message>
+	<message key="editor.submission.recommendation">Recommendation</message>
+	<message key="editor.submission.recommendation.description">Make a recommendation for the editors</message>


Recommend an editorial decision be made for this submission.

Do you maybe mean: Recommend an editorial decision for this submission.

Yeah that's better. :)

NateWr · 2017-08-29T10:33:01Z

templates/controllers/modals/editorDecision/form/recommendationForm.tpl

+	{if !empty($editorRecommendations)}
+		{fbvFormSection label="editor.submission.recordedRecommendations"}
+			{foreach from=$editorRecommendations item=editorRecommendation}
+				<div class="pkp_controllers_informationCenter_itemLastEvent">


Maybe remove the class name since it isn't used.

NateWr

Just the one comment on code review. I'll walk through it as a user now.

NateWr · 2017-09-06T09:37:59Z

js/controllers/list/submissions/SubmissionsListItem.vue

+					if (!this.activeStage.currentUserCanRecommendOnly) {
+						switch (this.activeStage.statusId) {
+							case pkp.const.REVIEW_ROUND_STATUS_RECOMMENDATIONS_READY
+							case pkp.const.REVIEW_ROUND_STATUS_RECOMMENDATIONS_COMPLETED


Missing the : at the end of the case statements. This is breaking your builds at the moment.

Wow, good catch! :-)

bozana · 2017-09-12T12:31:17Z

@NateWr, could you please take a look at the new commit "reload editor decision div" above -- it also generally provides the refreshOn for the load_url_in_div. Thanks to your global even feature! THANKS!!!

NateWr · 2017-09-12T13:30:46Z

js/controllers/grid/users/stageParticipant/StageParticipantGridHandler.js

@@ -40,6 +41,7 @@
 					.each(function() {
 						$.pkp.classes.Handler.getHandler($(this)).reload();
 					});
+			pkp.eventBus.$emit('decisionActionUpdated');


Just to make sure: here you're updating the decision div whenever a participant is added or removed. Is that right?

Yes, it is probably a very rare case, thus maybe also not needed: if one assigns himself/herself as a recommendOnly editor, he/she should see the "Make Recommendatin" button and the same if he/she removes himself/herself... Also if one assigns himself/herself as decision making editor, he/she should see the existing recommendations... -- I can also leave it so that such a user has to reload the page?
Or maybe also to check somehow if the user is on the review page and only then to reload it?

No that sounds good. Best to just reload it to keep the data fresh.

Hmmm... the #reviewDecisionsDiv-13 is reloaded in the line above but what that is... I have #reviewDecisionsDiv-9... Hmmm... And I see there seem to be a ways to reload those divs... as in the line above... I think I will have to test this once again... Sorry... :-\

And does this mean that this reload in the line above is bound to all 'dataChanged' events?

OK, this is not necessary here... I will change it...

bozana · 2017-09-12T14:21:47Z

Sorry @NateWr, that trigger of global event was not needed because that div should be reloaded in the earlier line, just that the div id was wrong. I fixed it, so could you please take another look at the same commit? Sorry, sorry, sorry!!! :-
I think we still need the refreshOn feature because if the change is coming from a page handler and there is no another/specific JS handler associated with the div, there is no possibility to upload it, correct?

NateWr · 2017-09-12T14:33:41Z

The code looks good. I haven't spun it up and tested, but I presume it's working alright.

bozana mentioned this pull request Aug 28, 2017

[OJS] Provide option to restrict low-level editors to "recommend" only, not make decisions #2433

Closed

NateWr suggested changes Aug 29, 2017

View reviewed changes

bozana force-pushed the 2433 branch from 67251f5 to 840aadc Compare September 5, 2017 18:07

NateWr suggested changes Sep 6, 2017

View reviewed changes

bozana force-pushed the 2433 branch 6 times, most recently from ca48fc8 to a5b4f8d Compare September 12, 2017 12:23

NateWr reviewed Sep 12, 2017

View reviewed changes

bozana force-pushed the 2433 branch from a5b4f8d to 6bc7d67 Compare September 12, 2017 14:17

bozana added 9 commits September 14, 2017 11:08

pkp#2433 editor recommendation

7f128be

pkp#2433 revisions according to the review

7f1c7b7

pkp#2433 fix missing colon in case statements

1f7cef4

pkp#2433 replace recommendation variable in the discussion message

dc99bfb

pkp#2433 fix js lint error

81131af

pkp#2433 fix SQL query for PostgreSQL

257f829

pkp#2433 consider internal review for OMP

a513baf

pkp#2433 reload editor decision div

036a47c

pkp#2433 recomendation test

e0e0a66

bozana force-pushed the 2433 branch from 6bc7d67 to e0e0a66 Compare September 14, 2017 09:24

bozana merged commit 9541cba into pkp:master Sep 14, 2017

bozana deleted the 2433 branch September 14, 2017 11:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkp/pkp-lib#2433 editor recommendation #2749

pkp/pkp-lib#2433 editor recommendation #2749

bozana commented Aug 28, 2017

NateWr left a comment

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

bozana Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

bozana Aug 29, 2017

NateWr Aug 29, 2017

NateWr Aug 29, 2017

NateWr left a comment

NateWr Sep 6, 2017

bozana Sep 6, 2017

bozana commented Sep 12, 2017

NateWr Sep 12, 2017

bozana Sep 12, 2017 •

edited

Loading

NateWr Sep 12, 2017

bozana Sep 12, 2017 •

edited

Loading

bozana Sep 12, 2017

bozana Sep 12, 2017

bozana commented Sep 12, 2017 •

edited

Loading

NateWr commented Sep 12, 2017

		$editDecisionDao = DAORegistry::getDAO('EditDecisionDAO');
		$editorRecommendations = $editDecisionDao->getEditorDecisions($submission->getId(), $this->getStageId(), null, $user->getId());

pkp/pkp-lib#2433 editor recommendation #2749

pkp/pkp-lib#2433 editor recommendation #2749

Conversation

bozana commented Aug 28, 2017

NateWr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NateWr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bozana commented Sep 12, 2017

Choose a reason for hiding this comment

bozana Sep 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bozana Sep 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bozana commented Sep 12, 2017 • edited Loading

NateWr commented Sep 12, 2017

bozana Sep 12, 2017 •

edited

Loading

bozana Sep 12, 2017 •

edited

Loading

bozana commented Sep 12, 2017 •

edited

Loading