digitalbazaar / vc

W3C Verifiable Credentials implementation in JavaScript
BSD 3-Clause "New" or "Revised" License
182 stars 52 forks source link

VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional #168

Closed aljones15 closed 7 months ago

aljones15 commented 11 months ago

Features:

  1. changes the DateRegexp to an XML schema DateTime and expands tests on it
  2. move expirationDate into the VC 1.0 validators and only checks date when mode is verify
  3. adds validUntil and associated tests
  4. adds validFrom and associated tests
  5. make some minor corrections with test data for better consistency
  6. This pr now contains credentialStatus check changes from here
  7. Makes credentialStatus.id optional
  8. Checks to ensure if credentialStatus.id is present it must be a URL.
codecov-commenter commented 10 months ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (update-vc-2.0@9ff44fe). Click here to learn what that means. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## update-vc-2.0 #168 +/- ## ================================================ Coverage ? 90.54% ================================================ Files ? 5 Lines ? 1058 Branches ? 0 ================================================ Hits ? 958 Misses ? 100 Partials ? 0 ``` ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/digitalbazaar/vc/pull/168?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=digitalbazaar). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=digitalbazaar) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/digitalbazaar/vc/pull/168?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=digitalbazaar). Last update [9ff44fe...0bb92ad](https://app.codecov.io/gh/digitalbazaar/vc/pull/168?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=digitalbazaar). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=digitalbazaar).
BigBlueHat commented 10 months ago

@aljones15 it looks like this PR has moved (or is moving) well beyond validUntil and validBefore. Is that correct? If so, we should relabel the PR at least.

I'm also a bit concerned about us ending up with one massive PR to review. Can we do more, smaller PRs against a non-main branch (as this one is against update-vc-2.0)? That would make reviews much faster, safer, and more sane. 😃

aljones15 commented 10 months ago

I will refrain from merging any further PRs into this PR, but as this PR and the other PR: https://github.com/digitalbazaar/vc/pull/170 had both been approved it seemed safe to merge the two.

As for this PR moving beyond validUntil & validBefore that was not intentional, but the feature making credentialStatus.id optional branched from the validUntil branch so it couldn't be aimed at the core VC 2.0 branch/PR.

BigBlueHat commented 10 months ago

I will refrain from merging any further PRs into this PR, but as this PR and the other PR: #170 had both been approved it seemed safe to merge the two.

As for this PR moving beyond validUntil & validBefore that was not intentional, but the feature making credentialStatus.id optional branched from the validUntil branch so it couldn't be aimed at the core VC 2.0 branch/PR.

Mostly, my aim is to keep topics clear...and now it's pretty muddied what's going on.

Also, as @JSAssassin mentioned earlier, the fields are validFrom and validUntil--and we should change this PR to match.

Thanks!

aljones15 commented 7 months ago

@BigBlueHat @davidlehn please double check this as this is your last chance before merge into update-vc-2.0.