Add Dwn.handleRecordsDelete function #549
Conversation
This commit introduces a handleRecordsDelete function meant to be used in place of the existing processMessage (which is eventually meant to be replaced, as described in issue decentralized-identity#289). This commit also updates the relevant tests in records-(write|delete|read).ts files, which were using processMessage and replaces its usage with the new handleRecordsDelete. It also introduces a test in dwn.spec.ts, enough to bring uncovered lines of this new function to 0 (more specifically, the new test is meant to cover the exception path).
shobitb
requested review from
mistermoe,
csuwildcat,
thehenrytsai and
diehuxx
as code owners
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
@thehenrytsai thanks for reviewing and approving! I merged it, but had to resolve a conflict. I think it needs another review. |
There was a problem hiding this comment.
Approved! So FYI, @diehuxx made a change (#554) which uses a clever alternate scheme to provide TS type info.
So we can put a hold on the rest of the methods if it turns out to be sufficient! The work might then end up being to remove all dedicated methods such as this one instead. But for now, the PR is good to merge!
|
Wow, nice! I learned something new about method overloading in TS today. That's indeed very clever @diehuxx! @thehenrytsai I don't have permissions to merge this above PR, but feel free to discard if you think that's the right direction anyway. Thanks. |
EbonyLouis
added
the
hacktoberfest-accepted
This commit introduces a handleRecordsDelete function meant to be used in place of the existing processMessage (which is eventually meant to be replaced, as described in issue #289).
This commit also updates the relevant tests in records-(write|delete|read).ts files, which were using processMessage and replaces its usage with the new handleRecordsDelete.
It also introduces a test in dwn.spec.ts, enough to bring uncovered lines of this new function to 0 (more specifically, the new test is meant to cover the exception path).