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

fix: use getTimeFormat for encounter date of service datetime picker #7203

Merged
merged 19 commits into from
May 27, 2024

Conversation

stephenwaite
Copy link
Sponsor Member

Fixes #7202

Short description of what this resolves:

Changes proposed in this pull request:

@bradymiller
Copy link
Sponsor Member

bradymiller commented Feb 5, 2024

hi @stephenwaite , fun stuff :)

  1. Would ensure the scripts that use this datepicker with time with formatinput turned also convert the time correctly to ensure no weirdness (both the php script and this javascript need to do the same thing)
  2. Also would incorporate these change in the new library/js/xl/jquery-datetimepicker-2-5-4-translated.js script (btw, i think there is a bug in that script if you follow the logic; hint: what happens if params.showSeconds is true and params.formatInput is false :) )

@stephenwaite
Copy link
Sponsor Member Author

testing well, thanks for the review, not sure about your hint but it made me take a deep 🤿

value='<?php echo attr(date('Y-m-d', time())); ?>'
title='<?php echo xla('yyyy-mm-dd'); ?>' />
<input class="datepicker form-control" type='text' size='16' class='datepicker' name='date_of_signature' id='date_of_signature'
value='<?php echo attr(oeFormatDateTime(date('Y-m-d H:i', time()), $GLOBALS['time_display_format'])); ?>'
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note if follow oeFormatDateTime to oeFormatTime, no need to set the 2nd parameter (if defaults to 'global' which then uses $GLOBALS['time_display_format'])

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that when formatInput is used the entry needs to be standardized at some point before going into the daatabase (via the DateTimeToYYYYMMDDHHMMSS() function ).
This form also has a discrepancy in it's view.php for this date widget.

Copy link
Sponsor Member

@bradymiller bradymiller Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So 2 things happen for internat to work on the datetimes.

  1. A standard datetime is converted to the localize format via oeFormatDateTime in the datepicker
  2. When saved or used in scripts it is converted back to the standard datetime via the DateTimeToYYYYMMDDHHMMSS function

(rec checking out DateTimeToYYYYMMDDHHMMSS use in code to get an idea how it works)

@@ -202,7 +202,7 @@ function provide_sum_pat(patient_id,encounter_id) {
<?php echo xlt('Begin Date'); ?>:
</td>
<td>
<input type='text' name='form_begin_date' id="form_begin_date" size='20' value='<?php echo attr(oeFormatDateTime($begin_date, 0, true)); ?>' class='datepicker form-control' />
<input type='text' name='form_begin_date' id="form_begin_date" size='20' value='<?php echo attr(oeFormatDateTime($begin_date, $GLOBALS['time_display_format'], true)); ?>' class='datepicker form-control' />
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in cases where you need to use the 2nd parameter (since need to set the second parameter to true, would just use the '"global" string instead of the actual global)

@@ -41,6 +41,8 @@
* @license https://github.com/openemr/openemr/blob/master/LICENSE GNU General Public License 3
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will need to ensure the new library/js/xl/jquery-datetimepicker-2-5-4-translated.js has same logic for the fixed time stuff

@adunsulag
Copy link
Sponsor Member

@stephenwaite I realize the other PR I brought in is now creating issues. Do you want this in 7.0.2.1 patch? Or do we bump to 7.0.2.2? I won't be able to revisit this until Wednesday if you need me to pull down and review.

@stephenwaite
Copy link
Sponsor Member Author

Hi @adunsulag. I think @bradymiller addressed that for me so I'll take another crack at this with the hopes of getting it in patch 1.

@stephenwaite stephenwaite added this to the 7.0.2.1 milestone May 21, 2024
@sjpadgett
Copy link
Sponsor Member

@bradymiller Would you take a quick look so maybe I can get in patch? Or i'll have to pull down and test

@bradymiller
Copy link
Sponsor Member

code looks great!

2DV

@sjpadgett sjpadgett merged commit 9a95f5d into openemr:master May 27, 2024
25 checks passed
sjpadgett pushed a commit to sjpadgett/openemr that referenced this pull request May 27, 2024
…penemr#7203)

* fix: use getTimeFormat for encounter date of service datetime picker

* php warning

* revert weno php warning

* forget formatTime just use format

* need time in format: too

* fix step

* validate on blur false prevents weirdness exiting picker

* fix work/school note

* fix other uses and some misc php styling

* php styling

* fixes from review

* misc php warning

* turn time off

(cherry picked from commit 9a95f5d)
@adunsulag adunsulag removed this from the 7.0.2.1 milestone May 28, 2024
@adunsulag
Copy link
Sponsor Member

Removed milestone, as the issue is now connected to the milestone to avoid duplicates.

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

Successfully merging this pull request may close these issues.

bug: encounter date of service time format
4 participants