Skip to content
This repository was archived by the owner on Jan 22, 2021. It is now read-only.

Using Mailer inside another package support #51

Closed
wants to merge 9 commits into from

Conversation

mcoenca
Copy link
Contributor

@mcoenca mcoenca commented Feb 1, 2016

This has not been tested on production, but I have an older version of exactly the same ideas that was working fine, and as this one works fine on development, others env should (ahem) be OK.

The idea is that inside the bundled folder in production (build), the packages assets are available through programs/server/assets/packages/packageAuthorName_packageName/pathToExportedAsset

We cannot access the package name from inside the package. So i just decided an easy way to do it would be to give all mail templates an optional packageFolderName field corresponding to packageAuthorName_packageName, and then add corresponding constants and methods to the chain of util.readFile etc...

Watch out if you are using Mailer inside a package and inside your main app. There is only one global Mailer variable, so you will override previous Mailer.config if you call it more than once.
But now you can call several Mailer.init ! (Btw @johanbrook I am not sure i did not break something by allowing to call Mailer.init several times: was your renaming and overwriting of the init function intentional ?)

At the end, Usage

/your_package/package.js

Package.describe({
  name: "bridges:notifications",
  summary: "Bridges notifications package",
  version: "0.0.1"
});
api.use([
     'lookback:emails'
]);
...
 api.addFiles([
    'lib/newQuestAnswer.html',
    'lib/newPatronedQuestAnswer.html', //various templates
    'lib/newBridgeComment.html'
    ], 'server', {isAsset: true}
  );
api.addFiles([
   'server/mailer_init.js'
], 'server')

/your_package/server/mailer_init.js

const NotifsEmailTemplates  = {
   newQuestAnswer: {
      path: 'lib/newQuestAnswer.html',
      packageFolderName: 'bridges_notifications',
      ....
  }, newPatronedQuestAnswer : {...}, newBridgeComment: {...}
}

Meteor.startup(() => {
  Mailer.init({
    templates: NotifsEmailTemplates,
   ....
   });
}

And then voila you can use Mailer.send({template: 'newQuestAnswer'})) from within the bridges:notifications package

And you can still use Mailer.init in your main app with different templates. (watch out for configs)

Best,

@johanbrook
Copy link
Contributor

Thanks for this! I'm gonna check it out in further detail, but it looks good at first look.

But please remind me: is Assets.getText() a no go for packages wanting to read files from private dir?

@mcoenca
Copy link
Contributor Author

mcoenca commented Feb 2, 2016

At the time I did this, I think it was a no go, but now in the Meteor docs at Assets there is this line

Note: Packages can only access their own assets. If you need to read the assets of a different package, or of the enclosing app, you need to get a reference to that package's Assets object.

So it should be ok to read everything with the Assets API now, but you may have to pass to the Mailer Template a reference to the upper package Assets Object

@kax0
Copy link

kax0 commented Mar 15, 2016

Hi,
I'm in the need of this feature, will this be merged?

if (!Package['chrisbutler:node-sass']) {
Utils.Logger.warn('Sass support is opt-in since lookback:[email protected]. Please add chrisbutler:node-sass from Atmosphere and try again.', TAG);
return Utils.readFile(scss);
}

const file = path.join(ROOT, scss);
const file = packageFolderName ?
path.join(PACKAGES_ROOT, packageName, relativePathFromApp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like packageName isn't defined here?

@johanbrook
Copy link
Contributor

Added some notes to the code.

@carlosbaraza
Copy link

👍

@carlosbaraza
Copy link

I am having issues with the SCSS files, because they are not found.

@carlosbaraza
Copy link

Hi @mcoenca, hopefully the PR I did to your branch will accelerate the merging of this branch.

Bests

@carlosbaraza
Copy link

By the way, I am currently using this in dev but I plan to put it in production later today. I will let you know if I find some other issue within it while developing or deploying.

@johanbrook
Copy link
Contributor

Great, thanks for testing.

@carlosbaraza
Copy link

@johanbrook @mcoenca, I am using the branch from where I did the PR to your branch in production and everything seems to be working just fine.

@carlosbaraza
Copy link

@johanbrook @mcoenca, the only thing might be missing is some documentation on the topic.

For future major version, I think might be better to refactor this to use just path to set the absolute path of the template, maybe still forcing people to set the affected files as assets (what I feel a bit hacky).

@mcoenca
Copy link
Contributor Author

mcoenca commented Mar 26, 2016

Sorry was on vacation, will answer your questions and merge carlos PR in following days. Best.

Bring branch to master HEAD and Fix CSS
@johanbrook johanbrook closed this Oct 31, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants