Jrod/dit 10045 reintroduce pull command with ns cli #116

jaerod95 · 2025-04-10T19:41:24Z

Overview

Restores support for parsing API tokens via a global config file.

Test Plan

Testing successfully completed in via:

Deleting your ~/.config/ditto file and then running the CLI should walk you through fetching and saving your api token.
Entering an invalid API Token should reprompt you to input a valid api token.
It should generate json files in your ./ditto/ directory in the repo when running pull, and pull down all projects with the base variant by default. This means a file for each project with the name of project-id___base.json
Setting some variants into your configuration should generate a different variant file per variant per project if variants exist for text items in that project for provided variants
Filtering by project ids should only fetch relevant text from projects.

… to the beta CLI - adds a `--legacy (-l)` CLI option that forces the application to run in legacy mode. - adds a new warning message when the CLI is run in legacy mode.

- Additionally removes the v8-compile-cache that provides little value in modern node applications of this size. - Adds minification support via esbuild as well as cleaner map files. - Updates package.json "files" property to now only ship required files to npm.

… CLI code

…ith-ns-cli

… on configuration. Update error handling in handleOutput to provide clearer output format messages. Add default value for generateDriverFile in ZI18NextOutput schema.

marla-hoggard

Hitting submit on the first half of the code review so you can start working on the feedback. I've made it through the outputs directory. Still need to look through services and utils directories.

lib/src/formatters/frameworks/i18next.ts

lib/src/formatters/json.ts

lib/src/http/checkToken.ts

lib/src/http/variables.ts

lib/src/index.ts

lib/src/outputs/shared.ts

…work class

…s/checkout, and actions/setup-node

…nment

marla-hoggard

Finished the code review! Comments are mostly small/nits, but the APIToken helpers need another pass, as the usage of the token parameter is inconsistent and not quite right (and currently includes type errors). I'll wait for you to update that set of methods before testing.

lib/src/formatters/json.ts

marla-hoggard · 2025-04-15T18:30:45Z

lib/src/formatters/mixins/javascriptCodegenMixin.ts

+    protected indentSpaces: number = 2;
+
+    protected sanitizeStringForJSVariableName(str: string) {
+      return str.replace(/[^a-zA-Z0-9]/g, "_");


True, just interesting that we chose to make our default a character that we don't allow and ultimately replace here 🤷‍♀️

marla-hoggard · 2025-04-15T18:46:40Z

lib/src/services/apiToken.ts

+export async function initAPIToken(
+  token: string | undefined = appContext.apiToken,
+  configFile: string = appContext.configFile,
+  host: string = appContext.apiHost


I see that this method is only called in one place, and in that place it's called without any parameters. Do we expect to update this implementation to call it elsewhere with parameters? If not, it might be best to just simplify and use the appContext values directly.

If we are going to keep the parameters, i would potentially change this to an object as well, since all three params are optional. I'd much rather see initAPIToken({ host: "ditto.com" }) than initAPIToken(undefined, undefined, "ditto.com").

Upon further inspection, i think you need to take another look at this method and the related token helpers. In my previous review, I noticed that checkToken was taking but totally ignoring a token parameter. You've since removed that unused parameter.

validateToken still takes a token parameter, but all it does is pass it into checkToken, which no longer accepts that, and then returns it. Meaning, that validateToken no longer needs to take a token parameter either.

Then the same is true in this method - it takes a token just to pass it into validateToken, which never uses that value.

This format of params with defaults is meant to aide when it comes to being able to test this method easily. It's a stylistic choice that already existed in the legacy implementation, and since this was a copy then modify scenario from the legacy codebase, it's included. I personally like the concept for testing as long as it's a consistent theme throughout the the implementation. For now, not planning on updating it.

lib/src/services/apiToken.ts

marla-hoggard · 2025-04-15T19:02:38Z

lib/src/services/apiToken.ts

+      logger.errorText(
+        "Something went wrong. Please contact support or try again later."
+      ) + eventStr
+    );


We do this same sentry code in several places. Might be nice to make a helper for it:

export const quitWithError = async (error: unknown, message?: string) => { const eventId = Sentry.captureException(error); const eventStr = `\n\nError ID: ${logger.info(eventId)}`; const errorMessage = message || "Something went wrong. Please try again later."; return await quit(logger.errorText(errorMessage) + eventStr); };

Then lines 64-71 could simply be:

return await quitWithError(error);

sounds like a great follow up improvement!

lib/src/services/globalConfig.ts

lib/src/services/projectConfig.ts

…default interceptor for axios requests and update token validation logic to improve error handling and context management.

lib/src/http/checkToken.ts

lib/src/services/globalConfig.ts

lib/src/http/client.ts

lib/src/services/projectConfig.ts

Co-authored-by: Marla Hoggard <[email protected]>

…return an empty object if parsing fails.

… github.com:dittowords/cli into jrod/dit-10045-reintroduce-pull-command-with-ns-cli

lib/src/http/checkToken.ts

Co-authored-by: Marla Hoggard <[email protected]>

… and update fetchText function to stringify filters in API request parameters.

…mproving code clarity and maintainability.

marla-hoggard

I've done a bunch of testing and things are working well! I do have one piece of feedback, which we could fix here or as a followup. The UX for inputting an invalid api key is a bit strange. When I enter a bad value, my initial line updates to say "token (value)" and then the new prompt is already pre-populated with my bad value and then an error message two lines below where my cursor is. It's just a bit confusing/overwhelming.

This is the state of the terminal immediately after I hit submit on "bad.key" the first time

jaerod95 added 11 commits April 7, 2025 11:10

Moves Legacy CLI into a legacy folder and provides a new entrypoint…

017bc22

… to the beta CLI - adds a `--legacy (-l)` CLI option that forces the application to run in legacy mode. - adds a new warning message when the CLI is run in legacy mode.

Add CONTRIBUTING.md file with guidelines for getting started with the…

3facdda

… CLI code

Adds IS_LOCAL flag for improved debugging experience in beta CLI

5422fe6

Adds support for fetching and saving API tokens via the beta cli.

ef27c86

Merge branch 'v5-beta' into jrod/dit-10045-reintroduce-pull-command-w…

35b51f5

…ith-ns-cli

Remove CONTRIBUTING.md file as part of project restructuring.

e109b1a

Cleanup file structure and prepare for tests

a936947

Introduce basic pull command

792433e

Full implementation of generating basic i18Next File

d5ec804

Enhance I18NextFormatter to conditionally generate driver files based…

c1a7269

… on configuration. Update error handling in handleOutput to provide clearer output format messages. Add default value for generateDriverFile in ZI18NextOutput schema.

jaerod95 requested a review from marla-hoggard April 14, 2025 16:25

jaerod95 assigned marla-hoggard Apr 14, 2025

Add support for json formats with frameworks

5756389

marla-hoggard reviewed Apr 15, 2025

View reviewed changes

jaerod95 added 5 commits April 15, 2025 09:19

Address PR feedback

eccd1a2

Add documentation for generateImportStatements method in I18NextFrame…

0ddb454

…work class

Update GitHub Actions to use latest versions of actions/cache, action…

e75b967

…s/checkout, and actions/setup-node

Update GitHub Actions workflow to use Ubuntu 24.04 for testing enviro…

d0fa86c

…nment

Remove obsolete test files for JSON and configuration services

4d82988

marla-hoggard requested changes Apr 15, 2025

View reviewed changes

jaerod95 added 2 commits April 15, 2025 13:00

Refactor API token handling and HTTP client configuration. Introduce …

eb1eee9

…default interceptor for axios requests and update token validation logic to improve error handling and context management.

add comments and small nit changes

ce9f3fa

marla-hoggard reviewed Apr 15, 2025

View reviewed changes

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

lib/src/services/globalConfig.ts Outdated Show resolved Hide resolved

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

marla-hoggard reviewed Apr 15, 2025

View reviewed changes

lib/src/services/projectConfig.ts Outdated Show resolved Hide resolved

jaerod95 and others added 5 commits April 15, 2025 13:20

Update lib/src/services/projectConfig.ts

faac6c9

Co-authored-by: Marla Hoggard <[email protected]>

Update lib/src/http/client.ts

2a438e0

Co-authored-by: Marla Hoggard <[email protected]>

Update lib/src/http/checkToken.ts

a5ac40f

Co-authored-by: Marla Hoggard <[email protected]>

Remove default data parameter from readGlobalConfigData function and …

3255e14

…return an empty object if parsing fails.

Merge branch 'jrod/dit-10045-reintroduce-pull-command-with-ns-cli' of…

eb311d6

… github.com:dittowords/cli into jrod/dit-10045-reintroduce-pull-command-with-ns-cli

marla-hoggard reviewed Apr 15, 2025

View reviewed changes

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

jaerod95 and others added 3 commits April 15, 2025 13:28

Update lib/src/http/checkToken.ts

4526b44

Co-authored-by: Marla Hoggard <[email protected]>

Refactor JSONFormatter to include a new line for improved readability…

7f3187b

… and update fetchText function to stringify filters in API request parameters.

Refactor JSONFormatter to use a variable for filename construction, i…

1f86da5

…mproving code clarity and maintainability.

marla-hoggard approved these changes Apr 15, 2025

View reviewed changes

jaerod95 merged commit b83b2c2 into v5-beta Apr 15, 2025
1 check passed

jaerod95 deleted the jrod/dit-10045-reintroduce-pull-command-with-ns-cli branch April 22, 2025 16:37

Jrod/dit 10045 reintroduce pull command with ns cli #116

Jrod/dit 10045 reintroduce pull command with ns cli #116

Uh oh!

Conversation

jaerod95 commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Test Plan

Uh oh!

marla-hoggard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marla-hoggard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marla-hoggard Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

marla-hoggard Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

marla-hoggard Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

jaerod95 Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marla-hoggard Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

jaerod95 Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

jaerod95 commented Apr 10, 2025 •

edited

Loading

jaerod95 Apr 15, 2025 •

edited

Loading