Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to run multiple processing strategies #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akarys92
Copy link

Adding message processing strategy that can run multiple other strategies. This allows non-TFS logging of bugs. Other Mail2bug projects can add their own custom connectors enabling scenarios like logging support to storage and even creating smart responders.

@msftclas
Copy link

msftclas commented Dec 25, 2017

CLA assistant check
All CLA requirements met.

@akarys92 akarys92 changed the title Initial commit Add ability to run multiple processing strategies Dec 31, 2017
Copy link
Contributor

@davisjms davisjms left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this is a good addition. I have added some comments, please take a look and we should be good to go.

Logger.InfoFormat("Initializing MessageProcessingStrategy");
return new SimpleBugStrategy(_config, workItemManager);

return new MultiStrategy(strategies);
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than creating a strategy to encapsulate others, can we update Mail2BugEngine's behavior to support a list of strategies instead of one. This will keep it simpler and will still support the same set of use cases. The default implementation can have only SimpleBugStrategy in the list.

@@ -187,6 +197,22 @@ private static TempFileCollection SaveAttachments(IIncomingEmailMessage message)
return attachmentFiles;
}

private IWorkItemManager InitWorkItemManager() {
IWorkItemManager workItemManager;
if (_config.TfsServerConfig.SimulationMode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason we need to duplicate this code here? We should rather keep this in Mail2BugEngine itself so that these classes are agnostic of which IWorkItemManager is being used.

_ackEmailHandler = new AckEmailHandler(config);
_messageToWorkItemMapper =
new MessageToWorkItemMapper(
_config.EmailSettings.AppendOnlyEmailTitleRegex,
_config.EmailSettings.AppendOnlyEmailBodyRegex,
_workItemManager.WorkItemsCache);
}

public SimpleBugStrategy(Config.InstanceConfig config, IWorkItemManager WitManager)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the same variable name as the original - workItemManager

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.

3 participants