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

Issue/1978 add merge tags now today yesterday #2143

Merged
merged 6 commits into from
Oct 2, 2024

Conversation

omarkasem
Copy link
Collaborator

@omarkasem omarkasem commented Sep 17, 2024

💾 Build file (97f8351).

@omarkasem omarkasem linked an issue Sep 17, 2024 that may be closed by this pull request
4 tasks
@omarkasem omarkasem self-assigned this Sep 17, 2024
@mrcasual
Copy link
Collaborator

@Mwalek, please test and ensure that it works properly with different timezones.

@Mwalek
Copy link

Mwalek commented Sep 17, 2024

@omarkasem @mrcasual

  • The merge tags all work, but I noticed that "now" shows the same value as "today." I'm unsure if this is expected.
Screenshot 2024-09-17 at 4 45 38 PM
  • The 'now', 'today', and 'yesterday' merge tags don't appear in the merge tag dropdown or search.
Screenshot 2024-09-17 at 5 12 04 PM
  • When the timezone is changed, the values for the 'now', 'today', and 'yesterday' merge tags are correct.
  • However, the timestamp modifier doesn't work correctly with timezones other than UTC/GMT. A UNIX timestamp is independent of timezones, so changing the timezone shouldn't affect the result. For example, when set to UTC+2, the time is incorrectly ahead by 2 hours.
Screenshot 2024-09-17 at 4 58 53 PM
  • The format modifier works as expected (even with different timezones).
Screenshot 2024-09-17 at 5 04 52 PM

Summary of Issues:

  • "Now" and "today" show the same value.
  • 'Now', 'today', and 'yesterday' merge tags are missing from the dropdown/search.
  • Timestamp modifier shows incorrect results for timezones other than UTC/GMT.

Test URL: https://gv-issue-2143.try.gravitykit.com/display-m-tag-view/ (test:test)

@rafaehlers
Copy link
Contributor

Should now output yyyy-mm-dd hh:mm:ss?

Also, the default output of today and yesterday should be a formatted date that can be used inside the Advanced Filters and/or [gvlogic] shortcode. @Mwalek did you test them on AF and [gvlogic]? If not, please do!

We need to be able to work on these conditions: https://docs.gravitykit.com/article/446-highlighting-entries-about-to-expire

@omarkasem
Copy link
Collaborator Author

@Mwalek @rafaehlers
1- The new tags act the same way as the today tag, so if you want to get the full value then you have to do {now:raw}
That the same thing that happens with today and date_updated etc..
2- I don't think we have to add them to the dropdown like gf does with today
3- I don't understand this one, What do you mean by unix timestamp? It works when i change the timezone in wordpress settings, how can i test that unix timestamp so i can test it?

I also tested it with gvlogic with now:timestamp and it worked

@Mwalek
Copy link

Mwalek commented Sep 18, 2024

@omarkasem

  1. Thanks for clarifying. {now:raw} works. Example output: 2024-09-19 00:46:11
  2. Noted.
  3. Please make reference to the following video: https://www.loom.com/share/328d3a93bb25402fbecad93769441832?sid=42d4aa6f-028c-44c9-bca6-c02bb8bb3554

@rafaehlers I'm still testing AF, but I can confirm that the new merge tags work with [gvlogic]. E.g:

[gvlogic if="{Due Date:3:mdy}" less_than="{today:format:m/d/Y}"]
	<span class="gk-task gk-task-expired">Expired</span>
[/gvlogic]

[gvlogic if="{Due Date:3:mdy}" is="{today:format:m/d/Y}"]
	<span class="gk-task gk-task-expiring-soon">Expiring Soon</span>
[/gvlogic]

[gvlogic if="{Due Date:3:mdy}" greater_than="{today:format:m/d/Y}"]
	<span class="gk-task gk-task-on-time">On Time</span>
[/gvlogic]

@omarkasem
Copy link
Collaborator Author

@Mwalek Thanks for the detailed video, That's fixed now with the latest commit

@Mwalek
Copy link

Mwalek commented Sep 19, 2024

Thanks @omarkasem!

@rafaehlers Do you want to propose any changes before I proceed to test the latest commit?

@mrcasual
Copy link
Collaborator

@rafaehlers, please see @Mwalek's comment.

@mrcasual mrcasual removed their assignment Sep 26, 2024
@rafaehlers
Copy link
Contributor

No, that's it, we're good @Mwalek 👍

@Mwalek
Copy link

Mwalek commented Sep 27, 2024

@omarkasem using 97f8351, the Advanced Filters "Add Condition" button doesn't show up, could you check?

### Steps to Reproduce
1. Install and activate 97f8351
2. Install and activate Advanced Filters
3. Create a NEW View
4. Visit the Filter & Sort Tab

Expected: The Add Condition button should be visible.

Actual: The Add Condition button is not visible.

Loom Video: https://www.loom.com/share/9d0bcd646a7c48958924e29e2c63ec81?sid=6f03f44a-3e20-403c-b4c9-49642625b356

@mrcasual
Copy link
Collaborator

mrcasual commented Oct 1, 2024

@Mwalek, was this fully tested?

@omarkasem, I see some outstanding items in this comment.

@Mwalek
Copy link

Mwalek commented Oct 2, 2024

@mrcasual needed to test a bit more. It's fully tested now. Couldn't spot any issues on PHP 7.4 and 8.3, even while using AF.

@omarkasem, please play attention to these changes as we keep
repeating most of them on your code. More specifically:

1) Use Yoda conditions
2) Function description should use third-person singular verb
to describe what it does (see https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/#documenting-tips)
3) Reuse variables if declared and do not declare variable if it's only used once
4) Pass only the required parameters to functions
@mrcasual mrcasual merged commit 3d3d81e into develop Oct 2, 2024
1 check passed
@mrcasual mrcasual deleted the issue/1978-add-merge-tags-now-today-yesterday branch October 2, 2024 15:06
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.

Add merge tags {now} {today} {yesterday}
4 participants