Chore: Add util tests and fix file tests #709
Conversation
|
Verified that @tonyjin has signed the CLA. Thanks for the pull request! |
src/lib/util.js
Outdated
| @@ -901,5 +902,6 @@ export function isValidFileId(fileId) { | |||
| * @return {boolean} Is Preview running in the Box WebApp | |||
| */ | |||
| export function isBoxWebApp() { | |||
| return (window.location.hostname || '').indexOf('app.box.com') !== -1; | |||
| const boxHostnameRegex = /(app|ent)\.(box\.com|boxcn\.net|boxenterprise\.net)/; | |||
| return !!Browser.getHostname().match(boxHostnameRegex); | |||
There was a problem hiding this comment.
Can you use regex.test here since you are returning a boolean?
src/lib/Browser.js
Outdated
| * | ||
| * @return {string} Window hostname | ||
| */ | ||
| static getHostname() { |
There was a problem hiding this comment.
This seems a bit out of place in this file to me. There is a few instances of window.location in the codebase, maybe create a Url class and we can refactor?
There was a problem hiding this comment.
If there's nothing else needed for URL related stuff, maybe just add it to util since it's being used in a few places?
There was a problem hiding this comment.
I specifically moved this here (instead of including in util.js) so I could test isBoxWebApp() without refactoring util to a class with all static functions or adding Babel Rewire. I'll move this to a new URL class per Daniel's recommendation.
tonyjin
force-pushed
the
add-fix-tests-2
branch
from
d06f0c4
to
d30e598
Compare
- Move hostname check to Browser function so we can test `isBoxWebApp()` - Fix incorrect `file.canDownload()` tests, they were missing brackets - Add missing peer dependency - Clean up package.json and run `yarn upgrade`
tonyjin
force-pushed
the
add-fix-tests-2
branch
from
d30e598
to
f8f0b11
Compare
isBoxWebApp()file.canDownload()tests, they were missing bracketsyarn upgrade