Skip to content

pass access token ttl value to wopi client#364

Merged
karakayasemi merged 1 commit intomasterfrom
access_token_ttl
Nov 20, 2020
Merged

pass access token ttl value to wopi client#364
karakayasemi merged 1 commit intomasterfrom
access_token_ttl

Conversation

@karakayasemi
Copy link
Contributor

@karakayasemi karakayasemi commented Nov 4, 2020

Wopi protocol suggests returning access_token_ttl value to wopi client. In this way, wopi client can decide what to do when token about to expire.
For example, collabora warns user when 15 minutes before expire and shows below message to user:
resim

If user still doesn't refresh the session, it warns again periodically in every 2 minutes.

Fixes owncloud/enterprise#4237

Copy link
Contributor

@mrow4a mrow4a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly how I would expect it to work. Great work! Unit tests not straightforward I belive?

@codecov
Copy link

codecov bot commented Nov 4, 2020

Codecov Report

Merging #364 (f489f0b) into master (e302823) will increase coverage by 3.02%.
The diff coverage is 31.25%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #364      +/-   ##
===========================================
+ Coverage      6.16%   9.18%   +3.02%     
  Complexity      307     307              
===========================================
  Files            14      14              
  Lines          1200    1197       -3     
===========================================
+ Hits             74     110      +36     
+ Misses         1126    1087      -39     
Impacted Files Coverage Δ Complexity Δ
lib/Controller/DocumentController.php 1.75% <0.00%> (-0.01%) 139.00 <0.00> (ø)
lib/Db/Wopi.php 63.88% <83.33%> (+63.88%) 5.00 <0.00> (ø)
lib/Db.php 15.66% <0.00%> (+15.66%) 33.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e302823...f489f0b. Read the comment docs.

@karakayasemi
Copy link
Contributor Author

There is no dependency injection in php classes. Also, touched classes have too many \OC::$server, static method usage etc. JS side has not single unit test. Yes, nearly imposible to test without big refactoring. Seems like test can be written only to newly added classes in this repository. Maybe @micbar can help to merge as it is.

@karakayasemi
Copy link
Contributor Author

@phil-davis Could you help for merging this pull request?

@karakayasemi karakayasemi force-pushed the access_token_ttl branch 5 times, most recently from 320d299 to fab6323 Compare November 20, 2020 20:08
@karakayasemi karakayasemi merged commit 81ed852 into master Nov 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the access_token_ttl branch November 20, 2020 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants