Update theme info command to support multiple environments #5392
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success2111 tests passing in 930 suites. Report generated by 🧪jest coverage report action from 7165d29 |
EvilGenius13
force-pushed
the
add-info-to-multi-env
branch
2 times, most recently
from
d744382 to
1f4d53f
Compare
| .map(([key, val]) => formatSection(key, `${val}`)) | ||
| .join('\n\n') | ||
| outputInfo(infoMessage) | ||
| const formattedInfo = await formatThemeInfo(output, flags) |
There was a problem hiding this comment.
Moved the logic of the output into packages/theme/src/cli/services/info.ts
| @@ -84,3 +101,54 @@ describe('info', () => { | |||
| }) | |||
| }) | |||
| }) | |||
|
|
|||
| describe('formatThemeInfo', () => { | |||
| beforeEach(() => { | |||
There was a problem hiding this comment.
Due to how our mocking works, we need to refresh when testing formatSection. I followed the same principles from another test.
| @@ -84,3 +85,26 @@ function tabularSection(title: string, data: InlineToken[][]): AlertCustomSectio | |||
| body: {tabularData: data, firstColumnSubdued: true}, | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Originally, I was going to try using tabularData but the main problem is the output is key : value so if you have PREVIEW_URL : https://mystoremyshopify.com?preview_theme_id=130327216212, it will wrap onto another line when within renderInfo.
There was a problem hiding this comment.
We could adopt an approach similar to the shopify theme dev command and use the LinkToken with a truncated label :)
There was a problem hiding this comment.
Agreed. I have taken this approach in the new commit.
| ], | ||
| }) | ||
|
|
||
| expect(formatSection).toHaveBeenCalledTimes(6) |
There was a problem hiding this comment.
How valuable of an assertion are you thinking this is? Can we skip mocking the function entirely?
There was a problem hiding this comment.
Thank you for this PR, @EvilGenius13!
I've noticed that we are not displaying the environment when users run the info command without passing the --theme:
Aside from that minor detail, the changes are on the right track! :)
EvilGenius13
force-pushed
the
add-info-to-multi-env
branch
2 times, most recently
from
2bccea7 to
f718064
Compare
This commit refactors the theme info command to use multiple environment flags.
EvilGenius13
force-pushed
the
add-info-to-multi-env
branch
from
f718064 to
7165d29
Compare
Follow up from #5284
Allow
theme infocommand to be called using multiple environmentsWHAT is this pull request doing?
These changes allow a user to pass multiple environment from their
shopify.theme.tomlfile at once.Before:
After:
With terminal hyperlinks:
Without terminal hyperlinks:
How to test your changes?
shopify.theme.tomlfileNote: You will need to create an access token through your shop(s) to bypass the need to login during theme commands.
The most basic example of the structure needed would be this:
theme info -e env1 -e env2or whatever you named your environments.Post-release steps
We will be converting more commands to support multiple environments.
Measuring impact
How do we know this change was effective? Please choose one:
Checklist