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

Date issues for non-default date_format_short in reviewDueDate and responseDueDate #3420

Closed
vasylOstrovskyi opened this issue Feb 23, 2018 · 8 comments
Assignees
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.

Comments

@vasylOstrovskyi
Copy link

vasylOstrovskyi commented Feb 23, 2018

What I do:

  1. Install fresh and clean OJS from git and configure as usual (with official tarball the same result). Single default en_US locale.
  2. Change date_format_short in config.inc.php to "%d.%m.%Y" - this is the only modification I do.
  3. Submit test article
  4. Send to review
  5. Add reviewer. Fields Response Due Date and Review Due Date are automatically filled with some reasonale default values (16.03.2018 and 23.03.2018). Click Add reviewer.

Result: In the list of assigned reviewers I have "Overdue" status with wrong date "Response due: 2016-03-20". Similarly for Review due. Dates in the database are set incorrect which implies their further wrong usage.
So I observe two issues:
i) wrong date. (this is similar to #2689, probably duplicate)
ii) wrong date format in Review status line (It would be better to use date_format_short there istead of %Y-%m-%d)

More investigations of possible reasons for that:
a. With Edit review action, the dates can be manually corrected and set correct.
b. If on the step 5 above I change dates manually or with datepicker, the dates are set and saved correctly, so the issue (i) is likely related to the initial setting of form data.

Update: on the edit review assignment page, the situation is the same: I can set and record correct dates, however, if I just press OK button (without modifying date fields, e.g. simply checking some details), these dates are again set to incorrect values (in my case, 20.03.2016 and 20.03.2023), which is very bad since in the form everything looked correct.

One more update: I investigate page source using browser debugger and narrowed the problem. I noticed that on load, the review edit page contains the following data for review due date field:

<div class="inline pkp_helpers_half">
	<input type="text" validation="required" class="field text datepicker required hasDatepicker" name="responseDueDate" value="03.03.2018" id="responseDueDate-5a9164e62d293">
	<input data-date-format="dd.mm.yy" type="hidden" name="responseDueDate" value="03.03.2018" id="responseDueDate-5a9164e62d293-altField">
	<span>...</span>
</div>

but any (manual or datepicker) modification of its value immediately change it to

<div class="inline pkp_helpers_half">
	<input type="text" validation="required" class="field text datepicker required hasDatepicker" name="responseDueDate" value="03.03.2018" id="responseDueDate-5a9164e62d293">
	<input data-date-format="dd.mm.yy" type="hidden" name="responseDueDate" value="2018-03-03" id="responseDueDate-5a9164e62d293-altField">
	<span>...</span>
</div>

so that the date format of value in the hidden field changed on modification.

@vasylOstrovskyi
Copy link
Author

Not sure that this is the right way, but for me the following patch worked for issue (i)

--- ojs-prev/lib/pkp/templates/form/textInput.tpl       2017-10-24 06:05:28.000000000 +0300
+++ ojs/lib/pkp/templates/form/textInput.tpl    2018-02-24 16:38:08.931273477 +0200
@@ -67,7 +67,7 @@
        {if $FBV_class|strstr:"datepicker"}
                <input data-date-format="{$dateFormatShort|dateformatPHP2JQueryDatepicker}" type="hidden"
                name="{$FBV_name|escape}{if $FBV_multilingual}[{$formLocale|escape}]{/if}"
-               value="{if $FBV_multilingual}{$FBV_value[$formLocale]|escape}{else}{$FBV_value|escape}{/if}"
+               value="{if $FBV_multilingual}{$FBV_value[$formLocale]|escape}{else}{$FBV_value|date_format:"%Y-%m-%d"|escape}{/if}"
                id="{$FBV_id|escape}{$uniqId}-altField" />
        {/if}

@defstat
Copy link
Collaborator

defstat commented Feb 24, 2018

@vasylOstrovskyi You have diagnosed the problem correctly. The -altField input has been added to hold the "real value" of the datepicker, independently of its "display value". The real value should be in a javascript "yy-mm-dd" or the "%Y-%m-%d" you have added, or else there are issues with the save-in-db process.

When the user chooses another value, the altFormat takes effect, and changes the value to the "yy-mm-dd" format. But if the user leaves the value as-is, and because the value comes in the short-date format from the server code, the altField sticks with the "wrong" value.

Would you care to add a Pull Request for this fix?

@defstat
Copy link
Collaborator

defstat commented Mar 5, 2018

@vasylOstrovskyi do you maybe want me to add the code you suggested?

@NateWr NateWr added the Bug:1:Low A bug that does not have a severe consequence or affects a small number of users. label Mar 6, 2018
defstat added a commit to defstat/pkp-lib that referenced this issue Mar 9, 2018
defstat added a commit to defstat/ojs that referenced this issue Mar 9, 2018
@defstat
Copy link
Collaborator

defstat commented Mar 9, 2018

@NateWr
PRs
pkp-lib: #3469
OJS: pkp/ojs#1870

@defstat
Copy link
Collaborator

defstat commented Mar 16, 2018

@NateWr reminder

@NateWr
Copy link
Member

NateWr commented Mar 19, 2018

Sorry for the delay, @defstat. Can you give me some steps to reproduce this issue so I can test it out?

@defstat
Copy link
Collaborator

defstat commented Mar 19, 2018

Not a problem @NateWr ...
The first comment starts with the steps to reproduce the issue. If you need more info, I will try to be more specific.

NateWr added a commit that referenced this issue Mar 20, 2018
[pkp-lib] #3420 Default Date Format added to datepicker
@NateWr
Copy link
Member

NateWr commented Mar 20, 2018

👍 All merged.

@NateWr NateWr closed this as completed Mar 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.
Projects
None yet
Development

No branches or pull requests

3 participants