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

Added Groups functionality #13

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dovetail-tech-user
Copy link

Updated to Allow Groups to be Added or Deleted. Add Contact to a Group. Get list of Contacts within a Group.

@dovetail-tech-user
Copy link
Author

Updated to Allow Groups to be Added or Deleted. Add Contact to a Group. Get list of Contacts within a Group.

@jackthorley
Copy link
Contributor

Hi John,

Apologies for the delay in reviewing this pull request.

I've given it a check over and it looks great, definitely a valuable contribution to the code base. However it seems as though there has been a merge issue - some files have been removed from the solution; namely the recent surveys code. Seems the files in question still remain in the folder structure, for example, so if you could include these back into the project we can finalise the code review and get it merged.

n.b We're hoping to get a Travis Pipeline setup which should help verify your code changes further.

If you need any assistance please drop us a comment!

Thanks,

Jack

try
{
var collection = groupService.GetGroups(_accountReference, PageIndex, PageSize);
var contacts = new PagedContactCollection();
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be inlined with the contact creation.

@Codesleuth
Copy link
Contributor

I've merged in #14. Can you rebase/merge from master and push again? You should see this pull request build on Travis.

@dovetail-tech-user
Copy link
Author

Hi Jack,

Sorry for not getting back to you on this. I will rebase/merge from master and push the changes again.

@RobinJDCox
Copy link

Hello again,

Have you got any feedback on the comments made earlier by Jack? If you're able to it would be good to address them and we'll have a look at accepting your pull request and merging it in.

Thanks again for your input,
Robin

@dovetail-tech-user
Copy link
Author

Hi Robin,

Sorry for the late reply I'm going to get this done.
-John

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.

4 participants