Skip to content

Separate service and viewmodel#107

Open
migig wants to merge 1 commit intoandrewdavey:masterfrom
migig:master
Open

Separate service and viewmodel#107
migig wants to merge 1 commit intoandrewdavey:masterfrom
migig:master

Conversation

@migig
Copy link

@migig migig commented Feb 21, 2015

We have a common base class for all viewmodels. So a viewmodel cannot inherit from that base class and from Postal.Email.

Seems others have this problem as well: see here, and here.

This pull request allows you to do this:

var viewName  = "~/Views/Foo/Index.cshtml";
var viewModel = new FooViewModel();
var service   = new EmailService();
service.Send(viewName, viewModel);

By adding overloads as I did, it also means that it's backwards compatible for those who want to carry on using it as is.

What do you think?

@migig migig mentioned this pull request Feb 21, 2015
@migig
Copy link
Author

migig commented Feb 21, 2015

I've since discovered another way to do this:

// create view model
var viewModel = new EmailViewModel();  // derives from a base class used by all view models
// populate the view model, etc.

// prepare the razor view template
var viewName = "~/Views/Emails/Template1.cshtml";
var email = new Postal.Email(viewName);
email.ViewData = new ViewDataDictionary(viewmodel);  // the trick that makes everything work

// render the template, and send the email
var service = new Postal.EmailService();
service.Send(email);

This works, but nonetheless, it would be better to have it in built into Postal.

At the very least, if you decide to reject the change, please add this approach to the documentation, so others can benefit? I'm also worried that the ViewData signature may change in the future, but if you document it then you won't be able to! ;)

@hanc2006
Copy link

Your trick works. I think it's better to add a new overloads for an easier use but not inside the email service class. The email service has to deal only to send the email correctly. The build of the email (viewmodel, data, sender, ...) must be done elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants