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

Patch date created: handle both Local and UTC dates #43

Merged
merged 20 commits into from
Jul 8, 2024

Conversation

maphew
Copy link
Contributor

@maphew maphew commented Jun 29, 2024

This patch is a bigger touch than the last one and I don't expect it to pass review without changes.

Patch_notes will now take the provided date, convert it to whichever of local or utc is missing, and set both properties on the target note. To use, call ea.patch_notes() using either of dateCreated or utcDateCreated optional parameters but not both. If timezone info is missing from the datetime object Patch_notes will use the tz of the local machine.

mydate= dateutil.parser.parse('1999.12.31') # or anything else that creates a datetime object
# use just one of these:
res = ea.patch_note(noteId, dateCreated=mydate)
res = ea.patch_note(noteId, utcDateCreated=mydate)

Example output for res = ea.patch_note(noteId, utcDateCreated=mydate) follows. Lines prefixed with --- are from my test script (so not emitted from trilium-py):

---parsed_date: 1899-01-15 00:00:00
utc date tzinfo was None, forced to UTC (1899-01-15 00:00:00+00:00)
Getting local_timezone...
timezone: Yukon Standard Time
Synchronized dates:
        local_date: 1899-01-14 17:00:00-07:00
        utc_date  : 1899-01-15 00:00:00+00:00
ETAPI Formatted date kind: local
        1899-01-14 17:00:00.143-0700
ETAPI Formatted date kind: utc
        1899-01-15 00:00:00.153Z
---Patched LOCAL response: 1899-01-14 17:00:00.143-0700
---Patched UTC response: 1899-01-15 00:00:00.153Z

The new functions are:

  • handle_dates(): Ensure that both local and UTC times are defined, and have same time (adjusted for timezone)
  • synchronize_dates(): Synchronize local and UTC dates. We expect only one of local or utc date to be passed, use that to define the other, and return both as datetime objects.
  • get_local_timezone(): get the timezone from the local machine

They could be collapsed into one function. I went back and forth a lot on that.

The pyproject.toml changes can be ignored/discarded. (After review I'll create a new branch with just the clean changes and a fresh PR.)

Known deficits:

  • using print instead of logger for reporting
  • No tests (I don't know how to write tests without an available Trilium Notes server to point them at.)
  • Readme not updated (waiting on code review)

I started this almost a full month ago and busted my brain several times sorting out a path that worked, and didn't reverse local/utc or some other logic error. I'm out of my comfort zone and will not be surprised at any mistakes pointed out!

@maphew
Copy link
Contributor Author

maphew commented Jun 29, 2024

My test script: patch-date-test.py.txt

@Nriver
Copy link
Owner

Nriver commented Jul 2, 2024

Thanks for the big PR! 😊

I have some feedback regarding your changes:

  1. Handling None Values for Dates:
    There could be cases where both dateCreated and utcDateCreated are None. For instance, if I need to correct a typo in the title and only want to change the title, I would use a command like this:

    ea.patch_note(
        noteId='some id',
        title="new title"
    )

    The handle_dates function should be updated to handle this scenario.

  2. Function Location:
    You can move the three functions to utils/time_util.py as they are static methods.

@maphew
Copy link
Contributor Author

maphew commented Jul 2, 2024

Thanks for the feedback! I was so deep in my local problem I overlooked that date changing is only sometimes the goal. Hah. I'll get on both of these suggestions.

@maphew
Copy link
Contributor Author

maphew commented Jul 5, 2024

  1. patching note without any date changes is allowed (0aff1cc)

  2. I've move all the new date functions except handle_dates() into time_util (cfa0670). I'm not that familiar with classes and static functions and utilities. This is my first time splitting code up like this so feel free to advise and critique.

@Nriver Nriver merged commit f1454a0 into Nriver:main Jul 8, 2024
@Nriver
Copy link
Owner

Nriver commented Jul 8, 2024

It works great. I've just made a new release.

@maphew
Copy link
Contributor Author

maphew commented Jul 9, 2024

oh! I expected some more changes needed. Wonderful that wasn't needed =)
commit 7d8611c removes the disabled code (won't change the release).

@Nriver
Copy link
Owner

Nriver commented Jul 10, 2024

I don't mind commented code; I have a lot of commented code in my own work. I can merge the clean-up PR if you'd like.

@maphew
Copy link
Contributor Author

maphew commented Jul 10, 2024

it can wait until my next contribution (I know what that is yet. I just know that tri-py is useful enough to me that I'm building workflows around it, and will want something else changed too)

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.

3 participants