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

Can't view item when client timezone is greater than loan window #93

Closed
dlasusa opened this issue Jan 11, 2022 · 8 comments
Closed

Can't view item when client timezone is greater than loan window #93

dlasusa opened this issue Jan 11, 2022 · 8 comments
Labels
Bug 🐛 Something isn't working Priority ★★★ High priority

Comments

@dlasusa
Copy link

dlasusa commented Jan 11, 2022

Running version: 0.4.1

We've had a strange issue come up where an end user located in Hong Kong tried to get a loan. Loan duration for the item is 3hrs.

The "Get Loan" button works and the additional popup about taking the loan also works, but when the Universal Viewer attempts to load the content it continually refreshes.

In the server logs we see (repeated over and over):
log(f'returning manifest for {barcode} for {anon(person.uname)}')

In the browser it keeps making a call to:
'/manifests/<barcode:int>'

When we have the end user change their timezone to Pacific (where we are located) the viewer is able to display the content as expected. Additionally, if we change the timezone on a test client locally to Hong Kong (we also tried Germany - so any timezone beyond the 3hr loan window) that also failed.

Changing the timezone on the server to GMT didn't seem to help.

Though we noticed with the server set to GMT, we were able to take loans out if the client was in the Pacific timezone. So as long as the client time is BEFORE the server time + loan duration, things work as expected.

We're unsure where the communication between the server and the browser is breaking down regarding the timezone/loan duration.

Any idea what might be causing this?

Thank you,
Dan

@mhucka mhucka added Bug 🐛 Something isn't working Priority ★★★ High priority labels Jan 11, 2022
@mhucka
Copy link
Contributor

mhucka commented Jan 11, 2022

I'm not 100% sure, but we may have had a user in Norway experience the same problem. (I say think beause it didn't occur to us to try to have them change the time zone, but the repeating-reload behavior sounds the same.) We think it was solved in version 0.5.x. Is it possible for you to update to that version? We do understand that the changes in 0.4→0.5 may require some work to update, but it might solve this particular problem.

@dlasusa
Copy link
Author

dlasusa commented Jan 12, 2022

Thanks! We'll try and do that. We've made a variety of changes to the 0.4.x code, so it might take some time. Have most of the changes for 0.5.x been in server.py?

@mhucka
Copy link
Contributor

mhucka commented Jan 12, 2022

Have most of the changes for 0.5.x been in server.py?

Unfortunately, no :-(. The changes touched almost everything, including the templates and settings.ini file.

Do your changes concern things that would be worth incorporating back into DIBS, or are they more about site customization? If they're general changes, I wouldn't mind finding out what they are, and trying to see if they could be incorporated into DIBS. That might be useful to other people and might reduce what you have to do to move to 0.5.

Otherwise, in terms of transitioning to 0.5, what comes to mind is the following approach: doing a diff of your current codebase against the unmodified 0.4 codebase to get a full list of your changes, then taking that list of changes and systematically walking down the list while looking at the 0.5 codebase, and (manually) working out how to replicate the changes in 0.5. That may sound painful, but my gut feeling is that it may not take that long to do, especially if done iteratively (do a pass to handle the really easy cases, cutting down the list, then do another pass for harder cases, again cutting down the list, and so on). I could try to help figure out the hardest ones where 0.5 changed so much it's not obvious how to recreate the change.

@mhucka
Copy link
Contributor

mhucka commented Jan 12, 2022

Just thinking further: if it would help, I could generate a diff of the 0.4 → 0.5 differences and try to write some kind of roadmap or guide.

@mhucka
Copy link
Contributor

mhucka commented Jan 12, 2022

I just reread your original bug report and just realized you described a way to repeat the error. I will check this in 0.5 tomorrow.

@dlasusa
Copy link
Author

dlasusa commented Jan 12, 2022

Looks like we may have found a fix.

We changed the uv.tpl from this:

// Calculate the delay to exiration (in msec) and force a reload then.
var now = new Date().getTime();

To:

// Calculate the delay to exiration (in msec) and force a reload then.
var now = new Date().toLocaleString('en-US', {timeZone: 'US/Pacific'}).getTime();

This seems to work for all of our test clients.

The only downside (though it doesn't seem to affect functionality) is that the "Loan End Time" in the viewer shows in Pacific time instead of the user's local time.

Hope that's helpful! Thank for your quick replies!

@mhucka
Copy link
Contributor

mhucka commented Feb 8, 2022

@dlasusa Very sorry about how long this took to get back to. I'm making this change in the code and will release a new version.

mhucka added a commit that referenced this issue Feb 8, 2022
This fix is thanks to @dlasusa reported 2021-01-12:
#93 (comment)
@mhucka
Copy link
Contributor

mhucka commented Feb 8, 2022

This is in commit 6e806b9.

@mhucka mhucka closed this as completed Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working Priority ★★★ High priority
Projects
None yet
Development

No branches or pull requests

2 participants