Skip to content
This repository has been archived by the owner before Nov 9, 2022. It is now read-only.

Improvements and small bug fix in List Webhooks extension methods #1233

Merged
merged 3 commits into from Jun 23, 2017

Conversation

ypcode
Copy link
Contributor

@ypcode ypcode commented Jun 22, 2017

Q A
Bug fix? yes
New feature? no
New sample? no

What's in this Pull Request?

  • Bug fix : Ensure List Id is loaded when needed
  • Pre-check of expiration date validity
  • Add a valid delta when validity period is 6 months exactly
  • better handling of exception in WebhooksUtility methods

@msftclas
Copy link

msftclas commented Jun 22, 2017

@ypcode,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@jansenbe
Copy link
Contributor

jansenbe commented Jun 22, 2017

Thanks Yannick, good change which I'll process.

@ypcode
Copy link
Contributor Author

ypcode commented Jun 22, 2017

You are welcome. It is my pleasure to contribute

@jansenbe
Copy link
Contributor

jansenbe commented Jun 22, 2017

I did have a peek at how we calculate the max expiration data and added some code snippet below. Can you adjust your PR to take today + 180 days as the max expiration. Internally we add a 90 minutes grace period to cater for time between the date being calculated and the webhook being added.

        internal const int expirationDateTimeMaxDays = 180;
        internal const int expirationDateTimeGraceMinutes = 90;

       DateTime utcNow = DateTime.UtcNow;
       // Maximum expiration enforced by this API includes a grace period
        DateTime maximumExpirationDateTime = utcNow
                .AddDays(SPWebhookSubscriptionInformation.expirationDateTimeMaxDays)
                .AddMinutes(SPWebhookSubscriptionInformation.expirationDateTimeGraceMinutes);

@ypcode
Copy link
Contributor Author

ypcode commented Jun 22, 2017

Done. I also added 2 unit tests to verify the very last valid expiration and a barely invalid one

@jansenbe jansenbe merged commit d540842 into pnp:dev Jun 23, 2017
@jansenbe
Copy link
Contributor

jansenbe commented Jun 23, 2017

Thanks. I've dropped the expirationDateTimeGraceMinutes from your code as we introduced that grace time for server side only.

@ypcode
Copy link
Contributor Author

ypcode commented Jun 23, 2017

@ypcode ypcode deleted the dev-webhooks-fix branch Jun 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants