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: support date formats with translated strings #4000 #4005

Merged
merged 13 commits into from
Feb 20, 2019

Conversation

ravinderk
Copy link
Collaborator

Description

This pr will resolve #4000

How Has This Been Tested?

https://www.useloom.com/share/21c6e1cbafe747a7afb8ca772c2f62a7

Screenshots (jpeg or gifs if applicable):

image

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

Copy link
Member

@DevinWalker DevinWalker left a comment

Choose a reason for hiding this comment

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

Seems like we're getting more complex for actually outputting the dates. Can this be simplified at all?

@@ -404,8 +404,8 @@ function give_form_search_query_filter( $wp ) {

$wp->query_vars['date_query'] =
array(
'after' => ! empty( $_GET['start-date'] ) ? give_get_formatted_date( $_GET['start-date'] ) : false,
Copy link
Member

Choose a reason for hiding this comment

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

What is the recommended usage of give_get_formatted_date() now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DevinWalker This function does not work fine with translated localize date strings.
I updated the function description:

b8edffc

@@ -621,11 +621,21 @@ public function date_filter_pre() {
$date_query = array();

if ( ! empty ( $this->args['start_date'] ) ) {
$date_query['after'] = give_get_formatted_date( $this->args['start_date'] );
$date_query['after'] = date(
Copy link
Member

Choose a reason for hiding this comment

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

Lots more code, can this be done in a dryer way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DevinWalker I did two tings here:

  1. since give_get_formatted_date does not work well with translated localized date strings, so we start using date fn to convert the date to MySQL date format which is standard.

  2. Added support for the timestamp in start_date and end_date query param.

* release/2.4.2:
  fix: process all possible export steps
Copy link
Member

@DevinWalker DevinWalker left a comment

Choose a reason for hiding this comment

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

Reviewed code and video and it looks good. Lots more logic but dates get complex through formats.

@DevinWalker DevinWalker merged commit 5954f1c into release/2.4.2 Feb 20, 2019
@DevinWalker DevinWalker deleted the issue/4000 branch February 20, 2019 21:37
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.

None yet

2 participants