Program for importing autotask contracts into dynamics CRM 2011

Question

We regularly have to convert a bunch of autotask Contracts to a custom CRM entity because we have employees who don't work in Autotask but need those contracts. I wrote the above code to handle this job. this code is supposed to run in a scheduled batch

1. it retrieves all CRM accounts;
2. it loops over all accounts, doing the following per account:
3. retrieve the account from autotask using the ID;
4. loop over all contracts for this account and do the following per contract;
5. see if the contract already exists in CRM (abonnement, because contracts were already taken);
6. if the contract already exists, modify it if necessary;
7. if the contract does not yet exist, calculate the price and create it in CRM.
8. once all contracts are processed, deactivate those which are expired.

I've already done an initial code review with a senior developer, whose changes I've already included in this version, but I'm wondering what other improvements might be made.

Some peculiarities: Autotask saves all fields as a string, but they're returned from the service as objects. Only the id field for an entity is not saved as a string, but as a long.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Net;
using System.ServiceModel;
using System.ServiceModel.Description;
using System.Text;
using Microsoft.Crm.Sdk.Messages;
using Microsoft.Xrm.Sdk;
using Microsoft.Xrm.Sdk.Client;
using AutoTask.Sync.autoTaskWebService;

namespace AutoTask.Sync
{
    public static class AutotaskAccountImporter
    {
        /// <summary>
        /// Main method that retrieves and updates.
        /// </summary>
        private static void Main()
        {
            //connect to the 2 programs
            AutoTaskConnector.ConnectToAutoTask();
            Crm.LoginToCrm();

            try
            {
                //get a list of accounts from CRM which are also in AutoTaskConnector
                List<Account> crmAccounts = Crm.GetAccountsWithAutotaskId();
                Console.WriteLine("{0} Accounts opgehaald uit CRM.", crmAccounts.Count);
                //prepare list of abonnementen to insert;
                List<acm_abonnement> abonnementen = new List<acm_abonnement>();
                int accountCount = 0;
                //prepare list of abonnementen to save;
                List<acm_abonnement> updatedAbonnements = new List<acm_abonnement>();
                //Loop over all accounts;
                foreach (Account crmAccount in crmAccounts)
                {
                    ExtractAbonnementenFromCrmAccount(crmAccount, ref updatedAbonnements, ref abonnementen, ref accountCount);
                }
                //create new abonnementen, update changed existing, deactivate those that have ended.
                Console.WriteLine("{0} Abonnementen wegschrijven naar CRM", abonnementen.Count);
                Crm.CreateAbonnementen(abonnementen);
                Console.WriteLine("{0} Abonnementen Aanpassen in CRM", updatedAbonnements.Count);
                Crm.UpdateBestaandeAbonnementen(updatedAbonnements);
                Crm.DeactivateVerlopenAbonnementen();
            }

            catch (FaultException<OrganizationServiceFault> ex)
            {
                Console.WriteLine("The application terminated with an error.");
                Console.WriteLine("Timestamp: {0}", ex.Detail.Timestamp);
                Console.WriteLine("Code: {0}", ex.Detail.ErrorCode);
                Console.WriteLine("Message: {0}", ex.Detail.Message);
                Console.WriteLine("Inner Fault: {0}",
                    null == ex.Detail.InnerFault ? "No Inner Fault" : ex.Detail.InnerFault.Message);
            }
            catch (SaveChangesException ex)
            {
                Console.WriteLine("The application terminated with an error.");
                Console.WriteLine("Message: {0}", ex.Message);
                Console.WriteLine("Inner Fault: {0}",
                    null == ex.InnerException ? "No Inner Fault" : ex.InnerException.Message);
            }
        }

        private static void ExtractAbonnementenFromCrmAccount(Account crmAccount, ref List<acm_abonnement> updatedAbonnements, ref List<acm_abonnement> abonnementen, ref int accountCount)
        {
            try
            {
                //get account from autotask;
                autoTaskWebService.Account autotaskAccount = AutoTaskConnector.GetAutotaskAccountForID(crmAccount.slfn_Klantnr.GetValueOrDefault());

                //Get all contracts for a certain account from autotask;
                IEnumerable<autoTaskWebService.Contract> autotaskContracts = AutoTaskConnector.GetContractsForAccount(autotaskAccount.id);
                int accountAbonnementenCount = 0;
                //Loop over all contracts
                foreach (autoTaskWebService.Contract autotaskContract in autotaskContracts)
                {
                    DateTime? nextInvoiceDateTime = null;
                    //get all contractserviceUnits
                    var servicescost = CalculatecostAndNextInvoice(autotaskContract, ref nextInvoiceDateTime);
                    //Check if the contract already exists in CRM
                    if (UpdateAbonnementIfNeeded(updatedAbonnements, autotaskContract, nextInvoiceDateTime, servicescost))
                    {
                        continue;
                    }
                    //create a new abonnement using the correct values;
                    abonnementen.Add(new acm_abonnement(crmAccount, autotaskAccount, autotaskContract, servicescost, nextInvoiceDateTime));
                    accountAbonnementenCount++;
                }
                if (accountAbonnementenCount > 0)
                {
                    Console.WriteLine("{0} abonnementen aangemaakt.", accountAbonnementenCount);
                }
                accountCount++;
                Console.WriteLine("{0} accounts verwerkt.", accountCount);
            }
            catch (AutotaskCallException autoexception)
            {
                Console.WriteLine("Foutmelding van Autotask API bij ophalen van {1}: {0}", autoexception.Message, autoexception.AutotaskEntity);
            }
        }

        private static bool UpdateAbonnementIfNeeded(List<acm_abonnement> updatedAbonnements, autoTaskWebService.Contract autotaskContract, DateTime? nextInvoiceDateTime, double servicescost)
        {
            List<acm_abonnement> bestaandeabonnementen = Crm.GetAbonnementenWithMultiversId(autotaskContract.id.ToString(CultureInfo.InvariantCulture));
            if (bestaandeabonnementen.Any())
            {
                //if it exists, check if the price, invoicedate or SLA have changed.
                //if they changed, update them and add them to the updatelist.
                acm_abonnement bestaandAbonnement = bestaandeabonnementen.First();
                Console.WriteLine("Abonnement bestaat al in CRM, checken of het geupdate moet worden.");
                if (bestaandAbonnement.ContractIsAltered(autotaskContract, nextInvoiceDateTime, servicescost))
                {
                    bestaandAbonnement.acm_bedrag = new Money((decimal) servicescost);
                    bestaandAbonnement.acm_volgendefactuurdatum = nextInvoiceDateTime;
                    bestaandAbonnement.acm_slapicklist = new OptionSetValue(Convert.ToInt16(autotaskContract.ServiceLevelAgreementID.ToString()));
                    updatedAbonnements.Add(bestaandAbonnement);
                    Console.WriteLine("{0} abonnementen dienen aangepast te worden.", updatedAbonnements.Count);
                }
                return true;
            }
            return false;
        }

        private static double CalculatecostAndNextInvoice(autoTaskWebService.Contract autotaskContract, ref DateTime? nextInvoiceDateTime)
        {
            List<ContractServiceUnit> contractServiceUnits = AutoTaskConnector.GetContractServiceUnitsForContract(autotaskContract.id);
            double servicescost = 0;
            if (contractServiceUnits.Any())
            {
                //calculate contract price and invoicedate based on retrieved units
                servicescost = contractServiceUnits.Sum(csu => Math.Round(Convert.ToDouble(csu.Price.ToString()), 2, MidpointRounding.AwayFromZero));
                nextInvoiceDateTime = DateTime.Parse(contractServiceUnits.First().EndDate.ToString()).AddDays(1);
            }
            else
            {
                //if no units found, try with services;
                List<ContractService> autotaskContractServices = AutoTaskConnector.GetContractServicesForContract(autotaskContract.id);
                if (autotaskContractServices.Any())
                {
                    servicescost = autotaskContractServices.Sum(acs => Convert.ToDouble(acs.AdjustedPrice.ToString()));
                }
            }
            return servicescost;
        }
    }
    /// <summary>
    /// Autotask interface
    /// </summary>
    public static class AutoTaskConnector
    {
        private static ATWS _oService;
        /// <summary>
        /// connects to AutoTaskConnector
        /// </summary>
        public static void ConnectToAutoTask()
        {
            const string sWebUri = "autotaskurl";
            _oService = new ATWS();
            NetworkCredential oCred = new NetworkCredential("autotaskusername", "autotaskpassword");
            CredentialCache credCache = new CredentialCache { { new Uri(_oService.Url), "Basic", oCred } };
            _oService.Url = sWebUri;
            _oService.Credentials = credCache;
        }

        /// <summary>
        /// Gets an account with a specific ID.
        /// Throws an autotaskException if Autotask returns an error.
        /// </summary>
        /// <param name="crmId">ID of the entity</param>
        /// <returns>A single Autotask Account with that CRM ID.</returns>
        public static autoTaskWebService.Account GetAutotaskAccountForID(int crmId)
        {
            StringBuilder sBuilderAccount = new StringBuilder();
            sBuilderAccount.Append(@"<queryxml version=""1.0"">");
            sBuilderAccount.Append(@"<entity>Account</entity>");
            sBuilderAccount.Append(@"<query>");
            sBuilderAccount.Append(@"<condition><field>AccountNumber<expression op=""Equals"">" + crmId +
                                   @"</expression></field></condition>");
            sBuilderAccount.Append(@"</query></queryxml>");
            ATWSResponse oResponseAccount = _oService.query(sBuilderAccount.ToString());
            if (oResponseAccount.Errors.Any())
            {
                throw new AutotaskCallException(oResponseAccount.Errors[0].Message, "Account");
            }
            if (!oResponseAccount.EntityResults.Any()) return null;
            autoTaskWebService.Account account = (autoTaskWebService.Account)oResponseAccount.EntityResults[0];
            return account;
        }
        /// <summary>
        /// Gets all contracts for a specific Account that are active today
        /// </summary>
        /// <param name="accountId">ID of the account</param>
        /// <returns>A list of Autotask Contracts bound to a certain account.</returns>
        public static IEnumerable<autoTaskWebService.Contract> GetContractsForAccount(long accountId)
        {
            string now = DateTime.Now.ToString("MM-dd-yyyy hh:mm:ss");
            StringBuilder sBuilderContract = new StringBuilder();
            sBuilderContract.Append(@"<queryxml version=""1.0"">");
            sBuilderContract.Append(@"<entity>Contract</entity>");
            sBuilderContract.Append(@"<query>");
            sBuilderContract.Append(@"<condition><field>AccountID<expression op=""Equals"">" + accountId +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"<condition><field>StartDate<expression op=""LessThanorEquals"">" + now +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"<condition><field>EndDate<expression op=""GreaterThanorEquals"">" + now +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"</query></queryxml>");
            ATWSResponse oResponseContract = _oService.query(sBuilderContract.ToString());
            if (oResponseContract.Errors.Any())
            {
                throw new AutotaskCallException(oResponseContract.Errors[0].Message, "Contract");
            }
            return oResponseContract.EntityResults.Cast<autoTaskWebService.Contract>().ToList();
        }

        /// <summary>
        /// Gets all services for a contract.
        /// Used when ServiceUnits are unavailable for whatever reason.
        /// </summary>
        /// <param name="contractId"></param>
        /// <returns>A list of ContractServices for a specific contract.</returns>
        public static List<ContractService> GetContractServicesForContract(long contractId)
        {
            StringBuilder sBuilderContract = new StringBuilder();
            sBuilderContract.Append(@"<queryxml version=""1.0"">");
            sBuilderContract.Append(@"<entity>ContractService</entity>");
            sBuilderContract.Append(@"<query>");
            sBuilderContract.Append(@"<condition><field>ContractID<expression op=""Equals"">" + contractId +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"</query></queryxml>");
            ATWSResponse oResponseContract = _oService.query(sBuilderContract.ToString());
            if (oResponseContract.Errors.Any())
            {
                throw new AutotaskCallException(oResponseContract.Errors[0].Message, "ContractService");
            }
            return oResponseContract.EntityResults.Cast<ContractService>().ToList();
        }

        /// <summary>
        /// Gets the serviceUnits for a contract.
        /// </summary>
        /// <param name="contractId"></param>
        /// <returns>A list of contractserviceUnits bound to a specific contract.</returns>
        public static List<ContractServiceUnit> GetContractServiceUnitsForContract(long contractId)
        {
            string now = DateTime.Now.ToString("MM-dd-yyyy hh:mm:ss");
            StringBuilder sBuilderContract = new StringBuilder();
            sBuilderContract.Append(@"<queryxml version=""1.0"">");
            sBuilderContract.Append(@"<entity>ContractServiceUnit</entity>");
            sBuilderContract.Append(@"<query>");
            sBuilderContract.Append(@"<condition><field>ContractID<expression op=""Equals"">" + contractId +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"<condition><field>StartDate<expression op=""LessThanorEquals"">" + now +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"<condition><field>EndDate<expression op=""GreaterThanorEquals"">" + now +
                                   @"</expression></field></condition>");
            sBuilderContract.Append(@"</query></queryxml>");
            ATWSResponse oResponseContract = _oService.query(sBuilderContract.ToString());
            if (oResponseContract.Errors.Any())
            {
                throw new AutotaskCallException(oResponseContract.Errors[0].Message, "ContractServiceUnit");
            }
            return oResponseContract.EntityResults.Cast<ContractServiceUnit>().ToList();
        }

    }

    /// <summary>
    /// Custom exception which gets thrown if an autotask API call returns an error.
    /// </summary>
    public class AutotaskCallException : Exception
    {
        public readonly string AutotaskEntity;

        public AutotaskCallException(string message, string entity)
            : base(message)
        {
            AutotaskEntity = entity;
        }


    }

    /// <summary>
    /// CRM interface
    /// </summary>
    public static class Crm
    {

        private static IOrganizationService _service;
        private static CRMContext _context;

        /// <summary>
        /// Logs into CRM with specific credentials.
        /// </summary>
        public static void LoginToCrm()
        {
            Uri organizationUri = new Uri("crmurl");

            var cred = new ClientCredentials();
            cred.UserName.UserName = "crmusername";
            cred.UserName.Password = "crmpassword";

            OrganizationServiceProxy serviceproxy = new OrganizationServiceProxy(organizationUri, null, cred, null);
            serviceproxy.ServiceConfiguration.CurrentServiceEndpoint.Behaviors.Add(new ProxyTypesBehavior());

            _service = serviceproxy;
            _context = new CRMContext(_service);
        }
        /// <summary>
        /// Get all accounts from CRM which have their autotaskID set.
        /// </summary>
        /// <returns>A list of all active CRM accounts that have an autotaskID.</returns>
        public static List<Account> GetAccountsWithAutotaskId()
        {
            return (from accounts in _context.AccountSet
                    where accounts.slfn_Klantnr != null
                    && accounts.StateCode == AccountState.Active
                    select new Account
                    {
                        slfn_Klantnr = accounts.slfn_Klantnr,
                        Name = accounts.Name,
                        Id = accounts.Id
                    }).ToList();

        }

        /// <summary>
        /// Creates all abonnementen from a list
        /// </summary>
        /// <param name="abonnementen">abonnementen to create.</param>
        public static void CreateAbonnementen(IEnumerable<acm_abonnement> abonnementen)
        {
            foreach (acm_abonnement acmAbonnement in abonnementen)
            {
                _context.AddObject(acmAbonnement);
            }
            _context.SaveChanges();
        }

        /// <summary>
        /// Checks if there already is an abonnement with a specific multiversID
        /// Prevents double creation of an abonnement.
        /// </summary>
        /// <param name="multiversId"></param>
        /// <returns>A list of abonnementen with a specific autotaskID.</returns>
        public static List<acm_abonnement> GetAbonnementenWithMultiversId(string multiversId)
        {
            return (from abon in _context.acm_abonnementSet
                    where abon.acm_multiversid == multiversId
                    select new acm_abonnement
                    {
                        acm_bedrag = abon.acm_bedrag,
                        acm_volgendefactuurdatum = abon.acm_volgendefactuurdatum,
                        acm_slapicklist = abon.acm_slapicklist
                    }).ToList();
        }

        /// <summary>
        /// Abonnementen which have expired (einddatum is earlier than the execution date) need to be deactivated.
        /// </summary>
        public static void DeactivateVerlopenAbonnementen()
        {
            List<acm_abonnement> verlopenAbonnementen = (from abon in _context.acm_abonnementSet
                                                         where abon.acm_einddatum <= DateTime.Now.Date
                                                         && abon.statecode != acm_abonnementState.Inactive
                                                         select abon).ToList();
            foreach (var acmAbonnement in verlopenAbonnementen)
            {
                SetStateRequest setAbonnementStateRequest = new SetStateRequest { EntityMoniker = new EntityReference(acm_abonnement.EntityLogicalName, acmAbonnement.Id), State = new OptionSetValue((int)acm_abonnementState.Inactive) };
                _service.Execute(setAbonnementStateRequest);
            }
        }

        /// <summary>
        /// Updates existing abonnementen
        /// </summary>
        /// <param name="updatedAbonnements">existing abonnementen</param>
        public static void UpdateBestaandeAbonnementen(IEnumerable<acm_abonnement> updatedAbonnements)
        {
            foreach (acm_abonnement updatedAbonnement in updatedAbonnements)
            {
                _context.UpdateObject(updatedAbonnement);
            }
            _context.SaveChanges();
        }
    }

    // ReSharper disable once InconsistentNaming
    public partial class acm_abonnement
    {
        /// <summary>
        /// Creates an abonnement based on the required CRM and Autotask entities.
        /// </summary>
        /// <param name="crmAccount"></param>
        /// <param name="autotaskAccount"></param>
        /// <param name="autotaskContract"></param>
        /// <param name="contractPrice"></param>
        /// <param name="nextInvoiceDate"></param>
        public acm_abonnement(Account crmAccount, autoTaskWebService.Account autotaskAccount, autoTaskWebService.Contract autotaskContract, double contractPrice, DateTime? nextInvoiceDate)
        {
            LogicalName = EntityLogicalName;
            acm_bedrag = new Money(Convert.ToDecimal(contractPrice));
            acm_begindatum = DateTime.Parse(autotaskContract.StartDate.ToString());
            //Autotask uses letters to indicate how often a contract is billed.
            if (autotaskContract.ContractPeriodType != null)
            {
                switch (autotaskContract.ContractPeriodType.ToString())
                {
                    case "m":
                        acm_contractperiodtype = new OptionSetValue(100000000);
                        break;
                    case "q":
                        acm_contractperiodtype = new OptionSetValue(100000001);
                        break;
                    case "s":
                        acm_contractperiodtype = new OptionSetValue(100000002);
                        break;
                    case "y":
                        acm_contractperiodtype = new OptionSetValue(100000003);
                        break;
                }
            }
            if (autotaskContract.ContractType != null)
            {
                //CRM optionset heeft zelfde waarden als autotask picklist.
                acm_contracttypepicklist = new OptionSetValue(Convert.ToInt16(autotaskContract.ContractType.ToString()));
            }
            acm_einddatum = DateTime.Parse(autotaskContract.EndDate.ToString());
            acm_klant = new EntityReference(Account.EntityLogicalName, crmAccount.Id);
            acm_multiversid = autotaskContract.id.ToString(CultureInfo.InvariantCulture);
            acm_naamklantautotask = autotaskAccount.AccountName.ToString();
            acm_name = autotaskContract.ContractName.ToString();
            acm_omschrijving = autotaskContract.Description.ToString();
            if (autotaskContract.ServiceLevelAgreementID != null)
            {
                acm_slapicklist = new OptionSetValue(Convert.ToInt16(autotaskContract.ServiceLevelAgreementID.ToString()));
            }
            acm_volgendefactuurdatum = nextInvoiceDate;
        }

        /// <summary>
        /// Checks if a contract has sufficiently been altered in Autotask.
        /// </summary>
        /// <param name="autotaskContract"></param>
        /// <param name="nextInvoiceDateTime"></param>
        /// <param name="servicescost"></param>
        /// <returns>true if any of the above field have been altered, false if they're all the same.</returns>
        public bool ContractIsAltered(autoTaskWebService.Contract autotaskContract, DateTime? nextInvoiceDateTime, double servicescost)
        {
            bool bedrag = (acm_bedrag.Value != (Decimal.Truncate((decimal)servicescost)));
            bool factuurdatum = false;
            if (acm_volgendefactuurdatum != null && nextInvoiceDateTime != null)
            {
                factuurdatum = (acm_volgendefactuurdatum.Value.ToLocalTime() != nextInvoiceDateTime.Value);
            }
            bool servicelevelagreement = ((acm_slapicklist ?? new OptionSetValue(1)).Value.ToString(CultureInfo.InvariantCulture) != (autotaskContract.ServiceLevelAgreementID ?? 1).ToString());
            return bedrag || factuurdatum || servicelevelagreement;
        }
    }
}

Answer 1

Magic Strings

string now = DateTime.Now.ToString("MM-dd-yyyy hh:mm:ss");
StringBuilder sBuilderContract = new StringBuilder();
sBuilderContract.Append(@"<queryxml version=""1.0"">");
sBuilderContract.Append(@"<entity>Contract</entity>");
sBuilderContract.Append(@"<query>");
sBuilderContract.Append(@"<condition><field>AccountID<expression op=""Equals"">" + accountId +
                       @"</expression></field></condition>");
sBuilderContract.Append(@"<condition><field>StartDate<expression op=""LessThanorEquals"">" + now +
                       @"</expression></field></condition>");
sBuilderContract.Append(@"<condition><field>EndDate<expression op=""GreaterThanorEquals"">" + now +
                       @"</expression></field></condition>");
sBuilderContract.Append(@"</query></queryxml>");

This code seems to be repeated enough to warrant an abstraction, be it only to get rid of inline XML string building.

This would require implementing an XmlQueryBuilder class and an XmlQueryOperator enum, but I'd like the query code to read something like this, so as to maximize maintainability and reduce the amount of possible typos and other hard-to-find bugs in the string:

var xml = new XmlQueryBuilder<Contract>()
            .Where(contract => contract.AccountID, XmlQueryOperator.Equals, accountId)
            .Where(contract => contract.StartDate, XmlQueryOperator.LessThanOrEquals, now)
            .Where(contract => contract.EndDate, XmlQueryOperator.GreaterThanOrEquals, now);

var result = _service.Query(xml.ToString());

The XmlQueryBuilder<TEntity>.ToString() method could use some Linq-to-Xml to produce the XML string in a strongly-typed way. In two words: avoid magic strings ;)

_{(I might implement such a class later, and put it up for review ;)}

---

Naming

It's not clear whether your API is French, English or German:

//Loop over all accounts;
foreach (Account crmAccount in crmAccounts)
{
    ExtractAbonnementenFromCrmAccount(crmAccount, ref updatedAbonnements, ref abonnementen, ref accountCount);
}

crmAccount and accountCount both sound English, updatedAbonnements is Frenglish, abonnementen sounds German, and ExtractAbonnementenFromCrmAccount looks like a mix of English and German, which isn't ideal. I'd try to stick to a single language, as much as possible. It looks like you have to cope with a somewhat multilingual API though.

The naming conventions aren't clear either. I'd stick to the C# conventions for everything I have control over, and use PascalCase for types and public/exposed members, and camelCase for locals and parameters - alllowercase isn't an option! Also, drop the random single-letter prefixes, like s on sBuilderContract - a typical name for a StringBuilder instance is just builder. oResponseAccount is also not clear about why it needs a "o" prefix (is that for "object?" - everything in .NET is an object!)...

---

Static

I think there's too much static going on here. It's static that blew up the Hindenburg - it's best to avoid static classes and methods, and to instantiate your service instead, so that you can reduce coupling and run unit tests that isolate and control these dependencies, if possible.