search

O365 / python-o365

A simple python library to interact with Microsoft Graph and Office 365 API
Apache License 2.0
1.6k stars 411 forks source link

New feature: add timezone to message Sent time #1043

Closed Elq-eng closed 5 months ago

Elq-eng commented 5 months ago

I had a problem, because when the library responded to a message, it did not respond with the time zone of the area, for example Bogotá Colombia, so I wanted to add a new feature.

Elq-eng commented 5 months ago

From what I understand, is the goal of this PR is to add the ability to inject (or modify) the timezone of the sent datetime?

More or less, the scenario I had was, when I tried to respond to a message with message.reply() it made the request but when I responded to it in my time zone it was 3:00 PM and in the email response it was 8 :00 PM

Invincibear commented 5 months ago

If you can, you should rename the PR to something like New feature: add timezone to message Sent time

Elq-eng commented 5 months ago

If you can, you should rename the PR to something like New feature: add timezone to message Sent time

Ok, please check

Invincibear commented 5 months ago

If you can, you should rename the PR to something like New feature: add timezone to message Sent time

Ok, please check

I see that the commit name changed, thanks, you should also change the name of the PR though. If you go to this PR on GitHub at the top you should be able to rename it.

Also, after you make these final changes can you show a successful test of the method so we know it doesn't have any errors?

Elq-eng commented 5 months ago

Well, please check out these pictures where the method is tested.

Before Before Now after

Elq-eng commented 5 months ago

LGTM?

alejcas commented 5 months ago

LGTM

Means Looks Good To Me. Common here on github

alejcas commented 5 months ago

There are a few issues with this PR.

First, yo can not inject (nor search for) the "Sent" keyword. This could be "Enviado" if the user uses spanish... So it will fail for other locales and also inject an incorrect keyword.

Other than that the problem with O365 it's that it was designed to always send to the server datetimes in UTC. That's why don't see the sent datetime like in your locale system settings. I will try to fix this so your problem is fixed as well by default.

alejcas commented 5 months ago

Thanks @Invincibear for the review process!

alejcas commented 5 months ago

After looking a bit into this I found that if when creating the reply you set the Prefer timezone to the desired timezone the time is displayed as it should be.

I will be implementing this in 2.1.0 release.

I'm sorry but can't accept this PR as it's to specific and will not be good to other users (language specific, datetime format specific. etc.)

Another approaches can be:

  1. create a new unrelated message and set all the properties to effectively be a reply of another message (conversation_id, etc.). This way you can configure the reply header in the message body.
  2. reply as it's done now but modify the body content to change whatever is needed (like you did in this PR)
alejcas commented 5 months ago

Thanks anyway for the effort!