feat: minor tweaks to embed-components and embed-services (#1315)

Author: josephaxisa

packages/embed-components/package.json

@@ -35,7 +35,7 @@
   "homepage": "https://github.com/looker-open-source/sdk-codegen/tree/master/packages/embed-components",
   "devDependencies": {
     "redux-saga-tester": "^1.0.874",
-    "@looker/sdk-node": "^23.8.1",
+    "@looker/sdk-node": "*",
     "@testing-library/react": "^11.2.7",
     "@looker/components-test-utils": "^1.5.27",
     "react-redux": "^7.2.9",
@@ -44,9 +44,10 @@
   },
   "dependencies": {
     "@looker/components": "^4.1.3",
-    "@looker/embed-services": "23.8.1",
+    "@looker/embed-services": "*",
     "@looker/redux": "^0.0.1",
-    "@looker/sdk": "^23.8.1",
+    "@looker/sdk": "*",
+    "@looker/sdk-rtl": "*",
     "@reduxjs/toolkit": "^1.9.3",
     "@styled-icons/material-outlined": "^10.47.0",
     "react": "16.14.0",

packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx

@@ -72,7 +72,7 @@ describe('QuickEmbed', () => {
     renderWithTheme(<QuickEmbed onClose={onClose} />)
 
     expect(
-      screen.getByRole('heading', { name: 'Get embed url' })
+      screen.getByRole('heading', { name: 'Get embed URL' })
     ).toBeInTheDocument()
     const textboxes = screen.getAllByRole('textbox')
 
@@ -106,7 +106,7 @@ describe('QuickEmbed', () => {
     renderWithTheme(<QuickEmbed onClose={onClose} />)
 
     expect(
-      screen.getByRole('heading', { name: 'Get embed url' })
+      screen.getByRole('heading', { name: 'Get embed URL' })
     ).toBeInTheDocument()
 
     expect(screen.queryByText(/Apply theme to/)).not.toBeInTheDocument()
@@ -127,7 +127,7 @@ describe('QuickEmbed', () => {
     renderWithTheme(<QuickEmbed onClose={onClose} />)
 
     expect(
-      screen.getByRole('heading', { name: 'Get embed url' })
+      screen.getByRole('heading', { name: 'Get embed URL' })
     ).toBeInTheDocument()
 
     expect(screen.getByText('Apply theme to dashboard URL')).toBeInTheDocument()

packages/embed-components/src/QuickEmbed/QuickEmbed.tsx

@@ -77,7 +77,7 @@ export const QuickEmbed = ({ onClose }: QuickEmbedProps) => {
   return (
     <Section padding="large">
       <Heading as="h3" fontWeight="medium">
-        Get embed url
+        Get embed URL
       </Heading>
 
       <SpaceVertical pt="medium" pb="medium" gap="xsmall">

packages/embed-services/src/EmbedUrl.ts

@@ -37,36 +37,41 @@ const THEMABLE_CONTENT: ContentType[] = [
 ]
 
 export interface IEmbedUrl {
+  /** Current url */
   readonly url: string
+  /** Path of the current page */
   readonly path: string
+  /** Search string of the current url */
   readonly searchParams: Record<string, string>
+  /** Determines whether the current URL is for a Dashboard */
   readonly isDashboard: boolean
+  /** Determines whether the current URL is for an Explore */
   readonly isExplore: boolean
+  /** Determines whether the current URl is for a Look */
   readonly isLook: boolean
+  /** Type of content the URL represents */
   readonly contentType: ContentType
+  /** Determines if current URL represents themable content */
   readonly isThemable: boolean
+  /**
+   * Builds the embed target url
+   * @param includeSearchParams switch determining whether to include search params from target url
+   * @param overrides any search key values to include in embed url e.g. 'k1=v1&k2=v2'
+   */
   embedUrl(includeSearchParams: boolean, overrides: Record<string, any>): string
 }
 
 /**
  * A class for use when implementer requires components to be context aware
  */
 export class EmbedUrl implements IEmbedUrl {
-  /** Current url */
   private _url: URL
-  /** Path of the current page */
   readonly path: string
-  /** Search string of the current url */
   readonly searchParams: Record<string, string>
-  /** Determines whether the current URL is for a Dashboard */
   readonly isDashboard: boolean
-  /** Determines whether the current URL is for an Explore */
   readonly isExplore: boolean
-  /** Determines whether the current URl is for a Look */
   readonly isLook: boolean
-  /** Determines if current URL represents themable content */
   readonly isThemable: boolean
-  /** Type of content the URL represents */
   readonly contentType: ContentType
   private readonly _embedUrl: string
 
@@ -106,11 +111,6 @@ export class EmbedUrl implements IEmbedUrl {
     return type
   }
 
-  /**
-   * Builds the embed target url
-   * @param includeSearchParams switch determining whether to include search params from target url
-   * @param overrides any search key values to include in embed url e.g. 'k1=v1&k2=v2'
-   */
   embedUrl(includeSearchParams = false, overrides: Record<string, any> = {}) {
     if (this.contentType === ContentType.Invalid) {
       throw new Error('Invalid content type')

packages/embed-services/src/ItemList.spec.ts

@@ -140,7 +140,10 @@ describe('ItemList', () => {
 
   describe('getCacheDefault', () => {
     it('gets the default', () => {
-      const actual = ItemList.getCacheDefault()
+      let actual = ItemList.getCacheDefault()
+      expect(actual).toBe(true)
+
+      actual = ItemList.getCacheDefault({ useCache: undefined })
       expect(actual).toBe(true)
     })
 

packages/embed-services/src/ItemList.ts

@@ -37,14 +37,31 @@ export interface GetOptions {
 export interface IItemList<T> {
   /** Cache time to live in seconds, defaults to 15 minutes */
   readonly timeToLive: number
+  /** Cached items */
   items: T[]
+  /** Expiration time */
   readonly expiresAt: number
+  /** Creates an indexed collection from the cached items */
   index(key?: keyof T): ItemList<T>
+  /** An indexed collection of items */
   indexedItems: Record<string, T>
+  /** Determines if the cache has expired */
   expired(): boolean
+  /** Computes the expiration time based on timeToLive */
   setExpiration(): void
+  /** Ejects cache if expired */
   clearIfExpired(): void
+  /**
+   * Searches the collection for an item with the specified key/value pair
+   * @param key or keys to search
+   * @param expression to match
+   */
   find(key: keyof T | Array<keyof T>, value: any): T | undefined
+  /**
+   * Gets the cache option value if present, otherwise defaults to true
+   * @param options to check
+   */
+  getCacheDefault(options?: GetOptions): boolean
 }
 
 export interface IEntityService<T> extends IItemList<T> {
@@ -58,11 +75,8 @@ export abstract class ItemList<T extends Record<string, any>>
   extends EntityService
   implements IItemList<T>
 {
-  /** Cached items */
   items: T[] = []
-  /** An indexed collection of items */
   indexedItems: Record<string, T> = {}
-  /** Time when cache expires */
   expiresAt = 0
   /** Key to index by */
   private keyField = 'id'
@@ -71,9 +85,6 @@ export abstract class ItemList<T extends Record<string, any>>
     super(sdk, timeToLive)
   }
 
-  /**
-   * Creates an indexed collection from the cached items
-   */
   index(key: keyof T = this.keyField) {
     this.keyField = key as string
     this.indexedItems = {}
@@ -85,33 +96,21 @@ export abstract class ItemList<T extends Record<string, any>>
     return this
   }
 
-  /** Computes the expiration time based on timeToLive */
   setExpiration() {
     this.expiresAt = Date.now() + this.timeToLive * 1000
   }
 
-  /**
-   * Determines if the cache has expired
-   */
   expired() {
     return this.expiresAt <= Date.now()
   }
 
-  /**
-   * Ejects cache if expired
-   */
   clearIfExpired() {
     if (this.expired()) {
       this.items = []
       this.indexedItems = {}
     }
   }
 
-  /**
-   * Searches the collection for an item with the specified key/value pair
-   * @param key or keys to search
-   * @param expression to match
-   */
   find(key: keyof T | Array<keyof T>, expression: string): T | undefined {
     let result: T | undefined
     let keys: Array<keyof T>
@@ -122,9 +121,8 @@ export abstract class ItemList<T extends Record<string, any>>
       keys = key as Array<keyof T>
     }
 
-    let rx: RegExp
     try {
-      rx = new RegExp(expression, 'i')
+      const rx = new RegExp(expression, 'i')
 
       for (const item of this.items) {
         for (const k of keys) {
@@ -141,12 +139,11 @@ export abstract class ItemList<T extends Record<string, any>>
     }
   }
 
-  /**
-   * Gets the cache option value if present, otherwise defaults to true
-   * @param options to check
-   */
   getCacheDefault(options?: GetOptions) {
-    const cache = options && 'useCache' in options ? options.useCache : true
+    const cache =
+      options && 'useCache' in options && options.useCache !== undefined
+        ? options.useCache
+        : true
     return cache
   }
 }

packages/embed-services/src/ServiceFactory.spec.ts

@@ -23,14 +23,14 @@
  SOFTWARE.
 
  */
-import { Looker40SDK as LookerSDK } from '@looker/sdk'
+import { functionalSdk40 as funSdk } from '@looker/sdk'
 import type { IAPIMethods } from '@looker/sdk-rtl'
 import { session } from './test-utils'
 import { createFactory, destroyFactory, getFactory } from './ServiceFactory'
 import { getThemeService, registerThemeService } from './ThemeService'
 
 describe('ServiceFactory', () => {
-  const sdk: IAPIMethods = new LookerSDK(session)
+  const sdk: IAPIMethods = funSdk(session)
 
   afterEach(() => {
     destroyFactory()

packages/embed-services/src/ServiceFactory.ts

@@ -28,7 +28,17 @@ import type { IAPIMethods } from '@looker/sdk-rtl'
 export type ServiceCreatorFunc<T> = (sdk: IAPIMethods, timeToLive?: number) => T
 
 export interface IServiceFactory {
+  /**
+   * Retrieves a service
+   * @param serviceName to retrieve
+   */
   get<T>(serviceName: string): T
+  /**
+   * Registers or creates a service
+   * @param serviceName name of service.
+   * @param serviceCreator function that creates the service.
+   * @param timeToLive in seconds, for the service cache. Defaults to 15 minutes.
+   */
   register<T>(
     serviceName: string,
     serviceCreator: ServiceCreatorFunc<T>,
@@ -51,12 +61,6 @@ class ServiceFactory implements IServiceFactory {
     return service
   }
 
-  /**
-   * Registers or creates a service
-   * @param serviceName name of service.
-   * @param serviceCreator function that creates the service.
-   * @param timeToLive in seconds, for the service cache. Defaults to 15 minutes.
-   */
   register<T>(
     serviceName: string,
     serviceCreator: ServiceCreatorFunc<T>,

packages/embed-services/src/ThemeService.spec.ts

@@ -24,7 +24,7 @@
 
  */
 import {
-  Looker40SDK as LookerSDK,
+  functionalSdk40 as funSdk,
   all_themes,
   update_theme,
   create_theme,
@@ -38,11 +38,10 @@ import { themeServiceCreator } from './ThemeService'
 import type { IThemeService } from './ThemeService'
 import { TestConfig, session, timeout } from './test-utils'
 
-const config = TestConfig()
-const themes = config.testData.themes
-
 describe('ThemeService', () => {
-  const sdk: IAPIMethods = new LookerSDK(session)
+  const config = TestConfig()
+  const themes = config.testData.themes
+  const sdk: IAPIMethods = funSdk(session)
   let service: IThemeService
   let testThemes: ITheme[]
   const themeCount = themes.length + 1 // includes the Looker theme

packages/embed-services/src/ThemeService.ts

@@ -41,7 +41,14 @@ export interface IThemeService
   extends IItemList<ITheme>,
     IEntityService<ITheme> {
   defaultTheme?: ITheme
+  /**
+   * Gets the default theme
+   * @param ts Timestamp representing the target datetime for the active period. Defaults to 'now'
+   */
   getDefaultTheme(ts?: Date, options?: GetOptions): Promise<ITheme>
+  /**
+   * Retrieves all themes and the default theme
+   */
   load(options?: GetOptions): Promise<IThemeService>
 }
 
@@ -101,10 +108,6 @@ class ThemeService extends ItemList<ITheme> implements IThemeService {
     return theme
   }
 
-  /**
-   * Gets the default theme
-   * @param ts Timestamp representing the target datetime for the active period. Defaults to 'now'
-   */
   async getDefaultTheme(ts?: Date) {
     if (this.expired()) {
       this.defaultTheme = await this.sdk.ok(default_theme(this.sdk, ts))
@@ -123,9 +126,6 @@ class ThemeService extends ItemList<ITheme> implements IThemeService {
     this.index()
   }
 
-  /**
-   * Retrieves all themes and the default theme
-   */
   async load(options?: GetOptions) {
     await this.getDefaultTheme()
     this.items = await this.sdk.ok(all_themes(this.sdk, options?.fields))