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

OCS Share API Date improvements #17166

Closed
rullzer opened this issue Jun 25, 2015 · 20 comments
Closed

OCS Share API Date improvements #17166

rullzer opened this issue Jun 25, 2015 · 20 comments

Comments

@rullzer
Copy link
Contributor

rullzer commented Jun 25, 2015

Currently the OCS Share API returns expiration dates as follows YYYY-MM-DD hh:mm:ss this is non practical for several reasons.

  • It is not a valid date according to ISO8601
  • It does not include timezone information (so it is always local server time)
  • hh:mm:ss is always 00:00:00

Now we could do two things.

Fix the date format

Move to proper ISO8601 dates. We could then show the time in UTC (YYYY-MM-DDThh:mm:ssZ) or in the actual timezone of the server (YYYY-MM-DDThh:mm:ss+hh:mm). The latter seems nicer.

The API could then also properly accept date and times using timezones.

Lets say my server is located in the Netherlands but I have users from the US on my server. If somebody from the US shares a file with an expiration date X then he would expect the same behaviour as if his server was located in the US. This would become possible using timezones.

Remove time

Since really we are only interested in Dates we could just leave the time.

However, then it would be a good idea to show the timezone of the server. So that clients can know when a share will expire. If the server has a different timezone than the client.

My opinion

At first I was very much in favour of removing the time. But the more I think about the the more the first solution appeals to me. However that would require some rethinking of the UX, maybe show the timezone in some way..

@jancborchardt from a UX point of view which one do you prefer?

CC: @DeepDiver1975 @PVince81 @schiesbn

@rullzer rullzer added this to the 8.2-next milestone Jun 25, 2015
@DeepDiver1975
Copy link
Member

We should use ISO8601 - it holds all necessary information and is easily parsable by e.g. Javascript - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/parse

@jancborchardt
Copy link
Member

I’d really prefer we don’t show the timezone anywhere in the interface. That will confuse people even more as no one can really wrap their heads around how timezones work. A day is good enough.

(If we actually do get complaints that the expiry date needs to be super exact, then we could do this: If the timezone is different from the local machine, we can show the exact expiry time (local time) next to the date.)

@PVince81
Copy link
Contributor

Isn't there another ISO format that only includes the date ?

Regarding timezones: if we ever have dates containing timestamps, we do need to use the proper ISO format when transferring them. However in the UI, they should be converted to the local timezone without timezone info, to be user-friendly. (we do that already for that dates of files, but not sure if they contain the timezone info)

@DeepDiver1975
Copy link
Member

We have to differentiate between data on the processing layer (which is where we need time and timezone) - on the ui we can display the data in any form.

@rullzer
Copy link
Contributor Author

rullzer commented Jun 25, 2015

@PVince81 ISO8601 also allows for date only.

@rullzer
Copy link
Contributor Author

rullzer commented Jun 25, 2015

@jancborchardt as @PVince81 mentions we can just display local time. But it is confusing when it displays expire on X at Y... while you can only set X.

@schiessle
Copy link
Contributor

But it is confusing when it displays expire on X at Y... while you can only set X.

It depends. It is right that you can only chose a day and we will expire the file as soon as the day starts (that's why the time is always 0:00). Actually it could be more confusing if you just say that the file will expire at 26 Jun. Because in this case you basically give a 24 hour window and a user could ask himself: when will the file expire on 26 Jun? In the morning? In the evening? Somewhere in between?

@rullzer
Copy link
Contributor Author

rullzer commented Jun 25, 2015

@schiesbn agreed. Altough, it does not solve the issue that it is always in server time. Which does not have to be the timezone of the user.

@DeepDiver1975
Copy link
Member

General design criteria for cross timezone applications is to

  • store all dates in UTC
  • convert dates back and forth according to the client's timezone

@schiessle
Copy link
Contributor

Altough, it does not solve the issue that it is always in server time. Which does not have to be the timezone of the user.

Yes, that's another fundamental question. We can only expire the file once. Assume following situation: Two people (userA and userB) from different timezones access a file with a expire data. For userA local time the expire date is already reached, but not for userB. What should the server do? Expire the share file or not? Should it show the file to userB but not to userA? When should the share be removed completely? If the last timezone passed the date?

Or should we use the timezone from the user who set the expire date as reference? What if the user travels? Let's assume he is on his way from Boston to Berlin and created a share in Boston right before he enter the plane which should expire the next day. Wouldn't it be quite confusing to always keep in mind that some shares now expire in Boston time and some in Berlin time depending on when the share was created during the trip?

I don't have a easy answer but I have the feeling that using the server time is a valid approach, especially as we can set the expire date only for full days. So every share who is set to expire on 26 Jun will expire at this day for each timezone. Not for everyone at 0:00 but at some point during the day and most important all shares with this expire date will expire at the same time which makes it more predictable.

@schiessle
Copy link
Contributor

General design criteria for cross timezone applications is to

  • store all dates in UTC
  • convert dates back and forth according to the client's timezone

a little bit off-topic, but it is not that easy. For example if you schedule a meeting and you cross summer/winter time between the day where you schedule the meeting and where the meeting take place. In this case it can happen that you schedule a meeting for 14:00 but at the end it will happen at 15:00 with this approach.

@rullzer
Copy link
Contributor Author

rullzer commented Jun 25, 2015

Still the 00:00:00 looks very weird. we can do better than that.

I would propose to send the date as ISO8601. So YYYY-MM-DDT00:00:00+hh:mm. So include the servers timezone and make sure it is a proper ISO8601 date. That way you clients can just display the date and maybe have some extra magic (on hover or something) that show that this is the servers timezone and in their local timezone this is at sometime on that date. Of course in most cases server and client have the same timezone so no additional magic is required.

@jancborchardt can we have something a bit easier on the eyes than the 00:00:00. Because I do agree with @schiesbn that just the date could mean at anytime on that day. But at the same time it really looks ugly. And inconsistent because you do not see it when you set the date.

@jancborchardt
Copy link
Member

(end of day)

@oparoz oparoz added the dev:ocs label Jun 25, 2015
@rullzer
Copy link
Contributor Author

rullzer commented Aug 29, 2015

So we should at least make sure this is properly done in OCS v2.

I'm not sure if changing this now to YYYY-MM-DD (so leaving out the time stuff) will work well on all users of the API.

@PVince81
Copy link
Contributor

@rullzer any update ? enhancement for 9.0 ?

@rullzer
Copy link
Contributor Author

rullzer commented Sep 22, 2015

9.0 since we would be breaking the API otherwise.

@rullzer rullzer modified the milestones: 9.0-next, 8.2-current Sep 22, 2015
@rullzer
Copy link
Contributor Author

rullzer commented Feb 16, 2016

Moving to 9.1 as we still cant' break the v1 API ;)
@PVince81 @cmonteroluque

@rullzer rullzer modified the milestones: 9.1-next, 9.0-current Feb 16, 2016
@ghost
Copy link

ghost commented Feb 17, 2016

@rullzer please don't move the milestone :). But yes, I will keep it where it is now

@PVince81 PVince81 modified the milestones: 9.1-current, 9.2-next Jun 14, 2016
@PVince81 PVince81 removed this from the 9.1-current milestone Jun 14, 2016
@PVince81
Copy link
Contributor

PVince81 commented Dec 8, 2016

Moving to backlog for now.

@PVince81 PVince81 modified the milestones: backlog, 10.0 Dec 8, 2016
@PVince81
Copy link
Contributor

PVince81 commented Dec 8, 2016

@DeepDiver1975 that is, unless you think it's critical and we can find someone with time to take care of this.

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

No branches or pull requests

8 participants