Added studio site set-domain and studio site set-https commands
#2151
Conversation
📊 Performance Test ResultsComparing 3f6193a vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
bcotrim
left a comment
bcotrim
left a comment
There was a problem hiding this comment.
Adding a custom domain for a running site isn't working for me, likely because await setupCustomDomain( site, logger ); is using the site from const site = await getSiteByFolder( sitePath, false ); that doesn't have a customDomain.
Cleaning up the logic to fetch the site (as I mentioned in the comments) should also fix this issue
cli/commands/site/set-domain.ts
Outdated
| const site = appdata.sites.find( ( site ) => arePathsEqual( site.path, sitePath ) ); | ||
| if ( ! site ) { | ||
| throw new LoggerError( __( 'The specified folder is not added to Studio.' ) ); | ||
| } |
There was a problem hiding this comment.
I don't think we need both getSiteByFolder and this validation, I understand we want to search the site in appdata to make the changes, so we could remove getSiteByFolder?
| @@ -0,0 +1,247 @@ | |||
| import { arePathsEqual } from 'common/lib/fs-utils'; | |||
| import { SiteCommandLoggerAction as LoggerAction } from 'common/logger-actions'; | |||
There was a problem hiding this comment.
lint shows warning 'LoggerAction' is defined but never used.
cli/commands/site/set-https.ts
Outdated
| try { | ||
| await lockAppdata(); | ||
| const appdata = await readAppdata(); | ||
| const site = appdata.sites.find( ( site ) => arePathsEqual( site.path, sitePath ) ); |
There was a problem hiding this comment.
Same as my comment on set-domain
Good catch 👍 I've updated the PR accordingly |
bcotrim
left a comment
bcotrim
left a comment
There was a problem hiding this comment.
Minor issue I noticed:
- When changing the domain of a site that already has a custom domain, we prompt for the admin password, twice.
We could avoid this by doing the change in one step "replace domain" instead of remove first and add later.
What do you think?
Code looks good and works as expected 👍
I agree, this is annoying. I created STU-1077 to fix this in a follow-up PR |
Related issues
Proposed Changes
studio site set-domaincommand, which updates the appdata file and restarts the site if running.studio site set-httpscommand, which updates the appdata file and restarts the site if running.Testing Instructions
npm run cli:buildnode dist/cli/main.js site set-https true --path PATH_TO_SITE(wherePATH_TO_SITEis the path to a Studio site WITH a custom domain)node dist/cli/main.js site start --path PATH_TO_SITEnode dist/cli/main.js site set-https false --path PATH_TO_SITEnode dist/cli/main.js site set-domain example.com --path PATH_TO_SITEnode dist/cli/main.js site set-domain example.local --path PATH_TO_SITEPre-merge Checklist