Page MenuHomePhabricator

[keyserver] Mark ids in input validators
ClosedPublic

Authored by michal on May 4 2023, 7:55 AM.
Tags
None
Referenced Files
F3387201: D7713.id26453.diff
Fri, Nov 29, 8:13 AM
F3387183: D7713.id26071.diff
Fri, Nov 29, 8:07 AM
F3387178: D7713.id26151.diff
Fri, Nov 29, 8:05 AM
F3386795: D7713.diff
Fri, Nov 29, 6:15 AM
Unknown Object (File)
Sun, Nov 24, 12:16 PM
Unknown Object (File)
Oct 28 2024, 8:38 AM
Unknown Object (File)
Oct 28 2024, 3:57 AM
Unknown Object (File)
Oct 14 2024, 5:35 AM
Subscribers

Details

Summary

We will also need to convert ids in the input, so this diff marks the ids for conversion inside existing input validators. tID should behave the same as t.String so there should be no runtime difference.

Depends on D7712

Test Plan

Went through all existing input validators and sub-validators and changed t.String to tID in places where ids should be updated.
Test if the web and mobile app still work.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

michal requested review of this revision.May 4 2023, 8:12 AM
tomek requested changes to this revision.May 5 2023, 12:47 AM

In diffs like this a test plan has to contain a description of what you have done to make sure that all the necessary changes were made. Without this it is impossible to verify that all the changes are there.

This revision now requires changes to proceed.May 5 2023, 12:47 AM
  • Updated the test plan
  • Also converted role ids. This required the conversion logic to handle tComb refinements, so I added that and tests
tomek added inline comments.
keyserver/src/utils/validation-utils.test.js
110 ↗(On Diff #26151)

Role value can be -1 - should we add a test for it?

This revision is now accepted and ready to land.May 8 2023, 3:01 AM
keyserver/src/utils/validation-utils.test.js
110 ↗(On Diff #26151)

I have tests with -1 role id in responder-validators.test.js