Layout support #8

derekrushforth · 2019-06-25T18:32:34Z

This pull request adds layout support to our templates pull and templates push commands. Looking for whatever feedback you guys have about the code and changes to the workflow. I'd love to release this by EOD Thursday (6/27).

templates pull

The folder structure downloaded from this command is a little different now. Templates and layouts are now organized into separate folders:

my-emails
├── layouts
│   └── plain
│       ├── content.html
│       ├── content.txt
│       └── meta.json
└── templates
    ├── password-reset
    │   ├── content.html
    │   ├── content.txt
    │   └── meta.json
    └── welcome
        ├── content.html
        └── meta.json

Changes to meta.json
There are a couple minor differences between the meta.json files for templates versus layouts.

templates - Has a new field called LayoutTemplate that lets people specify the layout alias being used for that particular standard template. It's not required. If users don't include this in the push command, we simply insert LayoutTemplate: null into the API request. This ensures that the layout is not changed at all. If you want to remove the layout being used on a template completely, pass an empty string to LayoutTemplate. This is how our public API works.
layouts - Only the Name and Alias are required. Does not include the Subject field.

templates push

This command now requires that the directory structure matches what you see above. Since this is a breaking change for anybody using the old version, I've bumped the major to 2.0.0.

Here's what pushing a template looks like now:

Two separate lists for templates and layouts. I kept it similar to the push screen in our UI.
You can see if a template is now using a different layout than what's on the server (see the welcome alias in the screenshot)
Minor tweaks to the copy to account for layouts

Tests

I've modified the tests a bit so they work with the new folders. The tests for the pull command also now check for an empty layout directory. This was only so I could get the tests to pass for now.

@ibalosh, can you tweak these so that we account for layouts on the test server?

cc @rianvdm

Also add some comments

error TS2345: Argument of type '(answer: { code?: string | undefined; }) => void' is not assignable to parameter of type '(value: unknown) => void | PromiseLike<void>'. Types of parameters 'answer' and 'value' are incompatible. Type 'unknown' is not assignable to type '{ code?: string | undefined; }’.

derekrushforth · 2019-06-25T18:40:48Z

src/commands/cheats.ts

@@ -48,7 +48,7 @@ const cheatInput = (hideMessage: boolean): Promise<string> =>
        choices: choices,
        message: hideMessage ? '\n' : title,
      },
-    ]).then((answer: { code?: string }) => {
+    ]).then((answer: any) => {


For some reason specifying a type on the answer value doesn't work in node 7.

ibalosh · 2019-06-26T10:24:42Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "postmark-cli",
-  "version": "1.0.0",
+  "version": "2.0.0",


I think this version bump is too high. First digit is reserved for major changes, we could probably go by something like 1.1.0 since there is a new feature being released.

Ah, though I just noticed you introduced it due folder structure.

Is there a way to do this without breaking existing integrations? Perhaps putting the "layouts" folder inside of the root download folder, or specifying the type as "Layout" within the metadata.json for the template? I think there are probably some other benefits to not splitting them into separate folders at the root.

I agree with not breaking existing integrations. Also my concern here is already having version 2 for app that is 1 month old. The feature is not that big in terms of CLI to justify that big change.

I do like though the folder split, since it follows the UI, and makes search through the list and separation easier. Although from what I see it would be easy to have them all in single folder since the name seems Alias based, therefore unique.

If we would like current new structure, there should be though an easy migration. All that would need to be done is check current folder structure (whether there is templates folder with subfolders) and provide a message to user that the files are migrated to new format.

I agree that 2.0.0 is a huge bump for something so new, so I was reluctant about doing this.

We should still strive for separation between standard templates and layouts somehow to make layouts easy to find and avoid additional configuration in meta.json. I often find myself needing to quickly do spot fixes in the layout, so this would be helpful. I suggest that we go with Andrew's suggestion and just add a layouts folder to the list of standard templates in the root, so it's not a breaking change.

my-emails ├── _layouts │ └── plain │ ├── content.html │ ├── content.txt │ └── meta.json └── password-reset │ ├── content.html │ ├── content.txt │ └── meta.json └── welcome ├── content.html └── meta.json

The reason for the underscore in the layouts folder name is so that it's more likely to appear at the top of the list. It's also to reduce the likelihood that it will conflict with existing aliases.

Thoughts?

Thanks for being flexible, @derekrushforth. I do think that as a convention, placing files in these folders

_layouts, _partials, etc. will be useful.

However, for a newcomer it may be non-obvious that the name of the folder is going to affect the type of template that gets saved. It also brings in another complication with moving templates, as changing the folder structure could cause the template type to change, which would be bad. If we think of each folder as being a "self contained" template, with all the info needed inside of it, maybe the problem gets easier...

By default, when we "pull" templates, this structure could work, but when "pushing" we're simply looking for directories that have the meta.json, content.html, and content.txt files.

From there, meta.json handles any extra properties that need to be included, including type, but the _layouts directory is purely a default convention we provide, rather than a requirement to maintain template information.

It's certainly more complicated to resolve all of the templates when pushing/pulling, but this would give a lot of flexibility in the customer grouping these templates into sub-groups, etc on their end.

@atheken definitely get what you're saying here. I like the idea of letting people change the folder structure to whatever they want, but yeah, this complicates things quite a bit.

My biggest concern is how we handle the process for pulling templates. We will need to figure out where each template is specifically located if they decided not to use our default folder structure.

I'm not so concerned about pushing, though. It's easier for me to rely on the template's metadata file instead of assuming template types based on the folder structure.

I'll give this a shot and let you know how it goes.

I think the big take away for the moment is that we don't "move" templates to new locations compared to the 1.0 release and we should include the template type in meta.json -- Standard would be assumed when not set.

Beyond that, defaulting to using a "_layouts" folder for new layouts is a good option.

We can layer on the "make your own folder structure later."

matt-west

The code looks good @derekrushforth! I tested pulling/pushing templates + layouts and it worked perfectly.

ibalosh · 2019-06-26T14:13:53Z

@derekrushforth added base tests which should be good for release, for stuff like pulling layouts, pushing them with changes, etc

Continue storing templates in root so that we don’t introduce breaking changes.

…ry structure - Read TemplateType field from meta.json - Ability to traverse any folder structure

derekrushforth · 2019-06-26T22:59:07Z

Hey folks,

I've updated the workflow based on the feedback. Here's a rundown:

TemplateType is now determined by what's specified in meta.json, instead of assuming based on the folder structure
Pushing no longer requires a strict folder structure. The command will traverse the folders so that you can organize templates and layouts however you want. As long as a single meta.json file and either HTML or Text content files are present in the same folder, it's good.
Pulling templates adds the layouts to a _layouts folder and standard templates will remain in the root directory. This is so that we do not introduce breaking changes. The only caveat is that we are not doing anything to detect whether the user has a custom folder structure. If you pull, then make major changes to your structure, then pull again, it's going to use our default structure. I'll look into introducing this as a separate feature release.

my-emails
├── _layouts
│   └── plain
│       ├── content.html
│       ├── content.txt
│       └── meta.json
└── password-reset
│   ├── content.html
│   ├── content.txt
│   └── meta.json
└── welcome
    ├── content.html
    └── meta.json

Version changed to 1.1.0
@ibalosh I also tweaked the tests so that they work with the new folder structure. Feel free to make any changes there.

Let me know what you guys think. If we're good, please approve the PR.

ibalosh

good to go @derekrushforth

derekrushforth added 16 commits June 19, 2019 16:57

Ability to pull and push layouts [WIP]

0d4fc52

Update tests [WIP]

b4fef10

Check if template or layout directory exists before parsing

6df5816

Also add some comments

Disable indentation errors in ESLint due to conflicts with Prettier

a1a7144

Improve process for pushing templates and layouts

e8e5d8a

Minor formatting tweak

84c7f5f

Improve review table when pushing

8fd74da

Remove unneeded types

a91c1b1

Update test

01f8990

Update types

09c5d2e

Update push confirmation screenshot

c30c98a

Bump version

d49d425

Update postmark.js package

575bf05

Fix test

ff63f99

Fix expected return value

b33f308

derekrushforth self-assigned this Jun 25, 2019

derekrushforth requested review from ibalosh and matt-west June 25, 2019 18:32

derekrushforth commented Jun 25, 2019

View reviewed changes

ibalosh reviewed Jun 26, 2019

View reviewed changes

matt-west approved these changes Jun 26, 2019

View reviewed changes

added layouts tests for pushes and pulls

0caa91f

derekrushforth added 5 commits June 26, 2019 10:26

Only store layouts in separate folder

140357e

Continue storing templates in root so that we don’t introduce breaking changes.

v1.1.0

e28e55d

Format with Prettier

613e79a

Update tests to work with new folder structure

1c01bab

Pushing templates no longer determines template type based on directo…

1c69bb7

…ry structure - Read TemplateType field from meta.json - Ability to traverse any folder structure

derekrushforth added 3 commits June 26, 2019 16:00

Clean up

43e684e

More clean up

9a6cfb5

Code refactor

bc57e61

ibalosh approved these changes Jun 27, 2019

View reviewed changes

derekrushforth merged commit a098d5f into master Jun 27, 2019

derekrushforth deleted the feature/layout-support branch June 27, 2019 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layout support #8

Layout support #8

derekrushforth commented Jun 25, 2019 •

edited

Loading

derekrushforth Jun 25, 2019

ibalosh Jun 26, 2019

ibalosh Jun 26, 2019

atheken Jun 26, 2019

ibalosh Jun 26, 2019

derekrushforth Jun 26, 2019

atheken Jun 26, 2019

derekrushforth Jun 26, 2019

atheken Jun 26, 2019

matt-west left a comment

ibalosh commented Jun 26, 2019

derekrushforth commented Jun 26, 2019 •

edited

Loading

ibalosh left a comment

Layout support #8

Layout support #8

Conversation

derekrushforth commented Jun 25, 2019 • edited Loading

templates pull

templates push

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matt-west left a comment

Choose a reason for hiding this comment

ibalosh commented Jun 26, 2019

derekrushforth commented Jun 26, 2019 • edited Loading

ibalosh left a comment

Choose a reason for hiding this comment

derekrushforth commented Jun 25, 2019 •

edited

Loading

derekrushforth commented Jun 26, 2019 •

edited

Loading