Implementation of "Users.GetIncrementalUserExport" and "Users.GetIncrementalUserExportNextPage" (+Async versions)

[rwjdk]

Implementation of "Users.GetIncrementalUserExport" and "Users.GetIncrementalUserExportNextPage" (+Async versions)

Requested in #315

UnitTests are a bit slow (All IncrementalExport are) but better to have them or not

[AppVeyorBot]

✅ Build ZendeskApi_v2 3.8.1-PullRequest347.15.build.254 completed (commit 614e9e5fd2 by @rwjdk)

[AppVeyorBot]

✅ Build ZendeskApi_v2 3.8.1-PullRequest347.16.build.254 completed (commit 4779bab176 by @rwjdk)

[mozts2005]

let's make this a DattimeOffset

[rwjdk]

Then it should also change in GroupTicketExportResponse (The class is based on that)... but that would be a breaking change so do not know if what is best. Make it right here and not make it consistent across the framework or have it same format. Up to you

[mozts2005]

It get what you are saying but let's make this class match the rest of the code by using DateTimeOffset and add an issue to fix the other.

I make breaking changes as needed. My goal with version 4 and above is to follow semantic versioning but for now, let's make the new code correct.

[mozts2005]

How do you plan on handling the rate limit in the test?

https://developer.zendesk.com/rest_api/docs/core/incremental_export#rate-limits
https://developer.zendesk.com/rest_api/docs/core/incremental_export#incremental-sample-export

May be we add a category to the test so if needed we can mark them as do not run? https://github.com/nunit/docs/wiki/Category-Attribute

[mozts2005]

Also, I am a fan fo the Assert.That() method of unit it test assertions.

[rwjdk]

Rate Limit
Do you run all tests that often? In my daily work, we have used the incremental Ticket Export and we are yet to hit the limit in actual use.
So gut feeling is wait and see if it becomes a problem and if so it could be built in that it have a "LastExecuted.txt" file in %temp% and if run too often.
But given that the test CanGetIncrementalTicketExportWithGroupsSideLoadPaged have not given an issue I guess this will not either (have same rate limit)

[rwjdk]

Regarding Assert.That

I can change my unit-tests to that (I normally use MS-Test and not nUnit and they do not have the Assert.That so not used to that)...

Practical how do I make this change to the PR? (Is it a new PR?) [Sorry: still very noob on this Git and PR thing]

[rwjdk]

Is this style OK (Assert.That feels clunky for null/not Null checks)?

        [Test]
        public void CanGetIncrementalUserExport()
        {
            var incrementalUserExport = api.Users.GetIncrementalUserExport(DateTimeOffset.MinValue);
            Assert.That(incrementalUserExport.Users.Count, Is.GreaterThan(0));
            Assert.IsNull(incrementalUserExport.Organizations);
            Assert.IsNull(incrementalUserExport.Identities);
            Assert.IsNull(incrementalUserExport.Groups);

            var incrementalUserExportNextPage = api.Users.GetIncrementalUserExportNextPage(incrementalUserExport.NextPage);
            Assert.That(incrementalUserExportNextPage.Users.Count, Is.GreaterThan(0));
            Assert.IsNull(incrementalUserExportNextPage.Organizations);
            Assert.IsNull(incrementalUserExportNextPage.Identities);
            Assert.IsNull(incrementalUserExportNextPage.Groups);
        }

[mozts2005]

@rwjdk for updating the PR just update the branch on your fork and it will auto update the PR

As for the test cases I like the

        [Test]
        public void CanGetIncrementalUserExport()
        {
            var incrementalUserExport = api.Users.GetIncrementalUserExport(DateTimeOffset.MinValue);
            Assert.That(incrementalUserExport.Users.Count, Is.GreaterThan(0));
            Assert.That(incrementalUserExport.Organizations, Is.Null);
            Assert.That(incrementalUserExport.Identities, Is.Null);
            Assert.That(incrementalUserExport.Groups,  Is.Null);

            var incrementalUserExportNextPage = api.Users.GetIncrementalUserExportNextPage(incrementalUserExport.NextPage);
            Assert.That(incrementalUserExportNextPage.Users.Count, Is.GreaterThan(0));
            Assert.That(incrementalUserExportNextPage.Organizations, Is.Null);
            Assert.That(incrementalUserExportNextPage.Identities, Is.Null);
            Assert.That(incrementalUserExportNextPage.Groups, Is.Null);
        }

The "That" format reads as English which helps new people to coding and is the format recommended by the NUnit team.

[rwjdk]

Fair point :-) - Change made to the tests now

[AppVeyorBot]

✅ Build ZendeskApi_v2 3.8.1-PullRequest347.18.build.254 completed (commit b6f6290786 by @rwjdk)

[rwjdk]

Requested changes are now updated in PR

[mozts2005]

Looks good thanks for all the changes.

[AppVeyorBot]

✅ Build ZendeskApi_v2 3.8.1-PullRequest347.19.build.254 completed (commit f928f2c825 by @rwjdk)