Email preview extension

[jwoertink]

This adds in the ability to view your carbon emails in the browser from within Breeze. The navbar link takes you to a list of the registered emails in your app with a button to view the HTML version or the TEXT version.

Here's some screenshots of how it looks:

Showing all registered emails

Viewing HTML email format

Viewing TEXT email format (out of date)

When your email doesn't have the format you're trying to view (in this case, TEXT) (out of date)

@paulcsmith, @matthewmcgarvey, @stephendolan - I'd love to get your input on both a design, and interface perspective.

[paulcsmith]

One quick thought: I was thinking there would be a way to view sent emails and not just preview ones. In the future when both are added I imagine Emails::Index would show 2 sections. Previews + sent emails. That would be really cool I think

Also, at the top of the Emails::IndexPage I think it'd be cool to link to docs on how to make email previews. That way people don't even need to search. They can just go straight to the docs on how to do this

[jwoertink]

@paulcsmith !!! I've missed you 😂 I like that idea. I was tossing around the idea between one or the other, but didn't think both lol. I think Carbon may need a few updates to really get that in so we can have pulsar send the sent email to breeze, but that shouldn't be too hard I think 🤔

[paulcsmith]

Yeah that was one I didn't get to. Pulsar could definitely emit an email sent, and then Breeze can hook into it and add it to an in-memory array. This was actually one of the main reasons for extracting Pulsar. So libs can hook into it as needed!

[jwoertink]

I feel a lot better about how this turned out. The last thing I would like to tackle before merging this in is, what do we do if you're not using Carbon? If you don't have Carbon required, then I'd imagine this wouldn't even compile. There isn't any sort of defined?(Carbon) type interface in Crystal.

My first thought is this gets abstracted out to a separate shard that essentially is the first "plugin" for breeze. But then when you install breeze, you'd also have to install plugin. I think that's probably fine... But how does that affect the Habitat config on Breeze? Can we append on to that? Is that a new config? Any other ideas?

[paulcsmith]

Lookin good!

I dig the installer if that's the direction we want to go, or to make it easy to add to old projects. We may want to consider bundling by default though. IMO it is something most projects will want

[paulcsmith]

In the future it might be good to expose this as a Habitat setting so people could add additional prefixes to ignore

settings.ignored_query_prefixes << "my_table"

[jwoertink]

Sweet!! I'm super stoked about how this is turning out. You now do not need have Carbon installed in order to use Breeze which makes this officially the first breeze extension, and it's opt-in.

How this works is you would just add your require:

require "breeze"
require "breeze/extensions/carbon"

This causes Habitat to add an additional required property to your breeze config to specify what the name of your email preview class is:

Breeze.configure do |settings|
  # The database to store the request info
  settings.database = AppDatabase

  # Enable Breeze only for this environment
  settings.enabled = Lucky::Env.development?

  settings.email_previews = Emails::Previews
end

And then you add in your email preview class:

# src/emails/previews.cr
class Emails::Previews < Carbon::EmailPreviews
  def previews : Hash(String, Carbon::Email)
    {
      "welcome"        => WelcomeEmail.new(UserQuery.first),
      "password_reset" => PasswordResetRequestEmail.new(UserQuery.first),
    } of String => Carbon::Email
  end
end

This is getting closer, but it's still not ready. The things I feel are left are:

• How to best document the preview setup
• Should the Carbon::EmailPreviews just go directly in to Carbon? 🤔 I'm ok with it being a patch for this....
• The navbar links as mentioned here or currently broken.
• A few cleanup deals I'd like to do, but aren't related to this (i.e. components called "SidebarLink", but our links are on top)
• This requires a new release of Habitat for the new Habitat.extend method.

[paulcsmith]

What do you think about instead passing a component rather than mounting it in the proc?

Suggested change

	setting links : Array(Proc(Breeze::SidebarLinks, Nil)) = [
	->(breeze : Breeze::SidebarLinks) {
	breeze.mount(Breeze::SidebarLink, context: breeze.context, link_text: "Requests", link_to: Breeze::Requests::Index.path)
	},
	->(breeze : Breeze::SidebarLinks) {
	breeze.mount(Breeze::SidebarLink, context: breeze.context, link_text: "Queries", link_to: Breeze::Queries::Index.path)
	},
	]
	end
	setting links : Array(Proc(Breeze::SidebarLinks, Nil)) = [
	->(breeze : Breeze::SidebarLinks) {
	Breeze::SidebarLink.new(context: breeze.context, link_text: "Requests", link_to: Breeze::Requests::Index.path)
	},
	->(breeze : Breeze::SidebarLinks) {
	Breeze::SidebarLink.new(context: breeze.context, link_text: "Queries", link_to: Breeze::Queries::Index.path)
	},
	]
	end

You can then mount the component. I can't remember if we removed the mount that lets you just give it a component class, but I think you should be able to do something like:

settings.links.each do |component|
  mount component
end

If we can't just give it a component then I'd suggest we add a version of mount that lets you give it a component instance

[jwoertink]

Yeah, we'd have to make a new mount method that takes a component instance. I think that would be cool though. Do you think this is fine as is until I can release a new Lucky? I could always come back and update later.

[jwoertink]

The more I dig in to this, the more I fight with the implementation. The above suggested change wouldn't work for a few reasons. 1. It needs a return type set. If it's Nil, then it wouldn't be allowed to be passed in to a mount, or even have methods called off it (i.e. component.call(self).view(view).render). 2. With how strict the Crystal Proc is, you can't define a base type, and allow it to return any sub-type. This doesn't compile:

setting links : Array(Proc(Breeze::NavbarLinks, Lucky::BaseCompoent)) = [
  ->(breeze : Breeze::NavbarLinks) {
     Breeze::NavbarLink.new(...)
  }
]

You end up getting the error:

web          |  5 | setting links : Array(Proc(Breeze::NavbarLinks, Lucky::BaseComponent)) = [
web          |                                                                               ^
web          | Error: class variable '@@links' of Breeze::NavbarLinks::HabitatSettings must be (Array(Proc(Breeze::NavbarLinks, Lucky::BaseComponent)) | Nil), not (Array(Proc(Breeze::NavbarLinks, Breeze::NavbarLink)) | Nil)
web          |

Now, if we return just a component instance, and it has to be a Breeze::NavbarLink instance, then the only reason we're passing in the breeze instance is to get access to context. We could shorten this whole structure by just asking "What's the title of your link, and where does it go?". The issue there is that it would be defined as Tuple(String, String) which doesn't really give you much information. Maybe it could be aliased as Tuple(LinkText, LinkLocation), but I'm not sure how well Crystal will play with the aliases there 🤔

The alternative is if we keep the definition as is, you have control to mount any component you'd like. The downside is that it's a little weird you have to call breeze.mount, and you have to know that everything is scoped to breeze breeze.context.

# setting links : Array(Proc(Breeze::NavbarLinks, Nil))
# This works with the code as is now.
Breeze::NavbarLinks.settings.links << ->(breeze : Breeze::NavbarLinks) {
  breeze.mount MyComponent
}
Breeze::NavbarLinks.settings.links << ->(breeze : Breeze::NavbarLinks) {
  breeze.mount Breeze::NavbarLink, ....
}

class MyComponent < Lucky::BaseComponent
  def render
    div "Static"
  end
end

[jwoertink]

oh! Just had an idea... pushing something up 😬

[jwoertink]

The beauty of using Crystal classes, is we can just make a method that does this for us! Not sure why I didn't think of this before, but now we have this:

# Use what we have built-in
Breeze::NavbarLinks.settings.links << ->(navbar : Breeze::NavbarLinks) {
  navbar.mount_link("Emails", to: Breeze::Emails::Index)
}

# Escape hatch to use your own custom component!
# This also means you can have a dropdown menu 😁 
Breeze::NavbarLinks.settings.links << ->(navbar : Breeze::NavbarLinks) {
  navbar.mount(MyCustomComponent, context: navbar.context, styles: "whatever")
}

[paulcsmith]

There may be some styles here:

breeze/src/breeze/pages/queries/index_page.cr

Line 27 in 2734273

link to: Queries::Show.with(query.id), class: "block hover:bg-indigo-50 focus:outline-none focus:bg-gray-50 transition duration-150 ease-in-out" do

that could help. Maybe mean we want to extract either the string class to a private method or maybe extract a component at some point

[jwoertink]

I'm not sure I follow here. I'm horrible with the design part ☹️ Maybe @stephendolan can hop in on this part?

[matthewmcgarvey]

We've talked about refactoring how breeze component/extensions (Requests, Database, Emails) integrate with breeze when it comes to things like navbar links. We're going to keep from letting issues like that block this.

Looks good to me :)

[jwoertink]

Thanks for the review! Yeah, I think once things are a little more fleshed out, we'll for sure be able to make an extension registration for future expansion.