Add rich text support for CLI #123

asnewman · 2025-08-08T23:51:25Z

Summary

Adds support for fetching and outputting rich text (HTML) content from the Ditto API.

Usage

Add richText: html to your configuration:

projects: [],
variants: [],
richText: html
outputs:
  - format: json,
    framework: i18next

It can be overridden with false

outputs:
  - format: json
    framework: i18next
    richText: false

When enabled, the CLI will output HTML-formatted text values in JSON files.

Testing checklist

Verify the base level richText: html works
Verify the overriding output config richText: false works
Verify the overriding output config richText: html works

First, run local Ditto and the public api - in ditto-app run your normal local start command + yarn start:api
Then, to run the CLI locally, run yarn start pull and paste in your API key if needed
You'll need to update ditto/config.yml to test the changes.
Check the json files to verify rich text output.

marla-hoggard

I'm going to push up a few commits to fix the filter issue, as well as update a prompt message that i noticed was out of date.

lib/src/http/textItems.ts

lib/src/services/apiToken/collectToken.ts

lib/src/http/textItems.ts

…ds/cli into ash/dit-11112-rich-text-for-cli

asnewman · 2025-08-14T22:06:33Z

@marla-hoggard for tomorrow, I added some tests for the pull function, should be good to rereview!

marla-hoggard · 2025-08-15T15:01:09Z

lib/src/commands/pull.test.ts

+const mockHttpClient = httpClient as jest.Mocked<typeof httpClient>;
+
+// Test data factories
+const createMockTextItem = (overrides: any = {}) => ({


Nit: Do we have these types defined in the CLI that we could use instead of any? Missing proper typing in tests makes them much less likely to be updated on changes/harder to write without making silly errors.

marla-hoggard · 2025-08-15T15:07:47Z

lib/src/commands/pull.test.ts

+
+// Create a temporary directory for tests
+let testDir: string;
+let outputDir: string;


Should these variables and the beforeEach/afterEach live inside the outer-most describe?

lib/src/commands/pull.test.ts

marla-hoggard · 2025-08-15T15:17:11Z

lib/src/commands/pull.test.ts

+
+const assertFilesCreated = (outputDir: string, expectedFiles: string[]) => {
+  const actualFiles = fs.readdirSync(outputDir).sort();
+  expect(actualFiles).toEqual(expectedFiles.sort());


Use the newer, non-mutating toSorted() instead of sort() for both of these to avoid directly mutating the arrays, potentially causing side effects in what else might be asserted after this line in the test.

marla-hoggard · 2025-08-15T15:31:28Z

lib/src/commands/pull.test.ts

+
+      // Mock should only return project-1 items when filtered
+      // The API only returns items for the requested projects
+      setupMocks([textItem1], []);


Unfortunately, these tests aren't actually testing the config filters at all. The only determining factor in what gets returned is what we pass into setupMocks. The filters are all used for determining the query params that we pass to the API endpoint. But here we're totally mocking the endpoint and manually providing response data. If I change the config values, the tests continue to pass.

These tests are doing a great job of validating the part of the flow from API response to CLI output, which works for the richText tests since that's where that logic gets applied, but the filters get applied in the config -> API request pathway, which is being 100% mocked right now.

To test the filters, we would want to inspect the request params that we send to the mocked API endpoint and confirm that we properly converted the config to the right params. That might be easier via directly testing the generateQueryParams method rather than doing it inside the pull command tests, but should hopefully also be accomplishable by inspecting what is sent to mockHttpClient if we want to keep it all in here.

I do think it's worth writing good filter tests, but I would be good if we removed them from this PR so we could get the richText functionality in and then opened a follow-up PR with the test updates. As long as we actually do it soon while the context is all in our head.

Ah good catch. I added a toHaveBeenCalledWith for the get call, that should test things properly, right? If not I'll just remove the test

Updates createMockTextItem function to use Partial<TextItemsResponse[0]> instead of any, providing better type safety for test mock overrides.

Improves test organization by moving testDir, outputDir variables and beforeEach/afterEach hooks inside the main describe block for better scoping and isolation.

Replaces mutating sort() calls with non-mutating toSorted() to avoid potential side effects in test assertions.

These tests mock HTTP calls and set appContext directly, so the YAML config files were never actually read. Removed the duplication between YAML strings and setProjectConfig() calls - now tests only create what they need and use a single source of truth for configuration. This eliminates the risk of YAML/object mismatch bugs and makes the tests clearer about what they're actually testing.

Adds toHaveBeenCalledWith assertions to filter tests to verify that config filters are correctly converted to API query parameters. Tests now catch bugs in the config->API params conversion logic instead of just testing the mocked response->file output flow. This addresses the concern that filter tests weren't actually testing the filtering functionality due to complete API mocking.

marla-hoggard

@asnewman I pushed some tweaks to the tests, adding a few more filter cases and splitting up the filter tests and the output file test, to more explicitly tie the tests to the parts of the flow that aren't mocked. Everything else looks good from my end!

asnewman added 15 commits August 8, 2025 16:28

Add richText field to TextItemsResponse schema

80bcd1b

Add richText configuration option to base output filters

2006fe5

Update Text Items API request to support richText parameter

3d2eb38

Pass richText configuration to API filters in JSON formatter

4200b0b

Use richText field in JSON output when configured and available

411de91

Add tests for richText field parsing in textItems

8bc78d7

Use both top level and out filter

fd44d16

formatting

9e6ccfc

remove unnecessary change

907b84a

remove spec files

35ebb5e

clarify code

828f0e8

simplify code

6a86fae

remove incorrect richtext param

45a4be0

add clarifying comment

c74c117

use boolean false instead of string

5d98a93

asnewman marked this pull request as ready for review August 12, 2025 23:18

asnewman requested a review from marla-hoggard August 13, 2025 23:49

asnewman assigned marla-hoggard Aug 13, 2025

marla-hoggard requested changes Aug 14, 2025

View reviewed changes

lib/src/http/textItems.ts Outdated Show resolved Hide resolved

lib/src/http/textItems.ts Outdated Show resolved Hide resolved

marla-hoggard added 3 commits August 14, 2025 12:33

Update url for finding your api key

26438da

Fix fetch text query params in fetchText

5cb3302

Move the changes higher up

3fc9598

marla-hoggard reviewed Aug 14, 2025

View reviewed changes

lib/src/services/apiToken/collectToken.ts Show resolved Hide resolved

marla-hoggard reviewed Aug 14, 2025

View reviewed changes

lib/src/http/textItems.ts Show resolved Hide resolved

marla-hoggard self-requested a review August 14, 2025 18:16

asnewman added 2 commits August 14, 2025 15:04

Add e2e tests

26820d9

Merge branch 'ash/dit-11112-rich-text-for-cli' of github.com:dittowor…

3809f50

…ds/cli into ash/dit-11112-rich-text-for-cli

marla-hoggard reviewed Aug 15, 2025

View reviewed changes

Replace any types with proper TypeScript types in tests

ee8a815

Updates createMockTextItem function to use Partial<TextItemsResponse[0]> instead of any, providing better type safety for test mock overrides.

asnewman added 4 commits August 15, 2025 14:41

Move test variables and hooks inside describe block

5347901

Improves test organization by moving testDir, outputDir variables and beforeEach/afterEach hooks inside the main describe block for better scoping and isolation.

Use toSorted() instead of sort() in tests

0a13b28

Replaces mutating sort() calls with non-mutating toSorted() to avoid potential side effects in test assertions.

asnewman requested a review from marla-hoggard August 15, 2025 22:22

Split up filter/output tests, add more filter cases

bb48ae2

marla-hoggard approved these changes Aug 18, 2025

View reviewed changes

asnewman merged commit 566a878 into v5-beta Aug 18, 2025
1 check passed

asnewman deleted the ash/dit-11112-rich-text-for-cli branch August 18, 2025 21:55

Add rich text support for CLI #123

Add rich text support for CLI #123

Uh oh!

Conversation

asnewman commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Usage

Testing checklist

Uh oh!

marla-hoggard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asnewman commented Aug 14, 2025

Uh oh!

marla-hoggard Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

marla-hoggard Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marla-hoggard Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

marla-hoggard Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

marla-hoggard Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

asnewman Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

marla-hoggard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asnewman commented Aug 8, 2025 •

edited

Loading