Skip to content

Replace inheritance with composition in Settings Diagnostics

Problem to Solve

!2518 (merged) was merged in an interest of time but there is an inheritance pattern being used in the code is making the code more confusing than it needs to be.

Proposed Solution

Use composition to make the code a bit friendlier to read and make testing more intentional.

Nice to have: Simplify the way diagnostics configuration is created so that the tests for settings diagnostics are not as lengthy with repeated configuration mocking.

Technical Implementation Details

I suggest creating this abstraction:

type SettingsSectionRenderer = (details: SettingsDetails) => string;

Each section of the settings will be rendered as this function. These can be stateless, not part of a class.

We will create three sections in common:

  • feature flags
  • duo pro
  • other

These will be 3 functions that will be easy to test and they'll be directly imported by the desktop and browser renderer.

I imagine the renderer class to look like this:

import { renderFeatureFlags, renderDuoSettings, renderOther } from '../common/...';


const renderCustomCertifices = (details) => return `string`;\
const renderCodeSecurity = (details) => return `string`;

class implements DesktopSettingsStateDiagnosticsRenderer DiagnosticsRenderer<[SettingsDetails]> {
  render([settingsDetails]: [SettingsDetails]): DiagnosticsSection[] {
    const sectionRenderers = [
      renderFeatureFlags,
      renderDuoSettings,
      renderOther,
      renderCustomCertifices,
      renderCodeSecurity,
    ];
    return [{
      title: 'Duo Workflow Settings',
      content: sectionRenderers.map(sr => sr(settingsDetails)).join('\n'),
    }];
  }
}

/cc @dbernardi

Edited by Dylan Bernardi