Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new utility function to verify if a user's profile meets required venue criteria, enabling a consolidated check within tools.js.
- Introduces the isProfileComplete method to validate profile properties against defined requirements.
- Implements logic to traverse nested profile fields and conditionally validate based on expected value types.
| isProfileComplete(profile, profileReqs) { | ||
| // If no profile or no requirements, skip check | ||
| if (!profile.id.startsWith('~') || !profileReqs) { | ||
| return true; |
There was a problem hiding this comment.
Shouldn't you throw an error if a profile object is not passed?
There was a problem hiding this comment.
Decided to not check profile.id because the other tools functions that handle profiles don't do this and I think it should be handled at the call site.
| * @param {object} profileReqs - An object defining the required profile fields and conditions. | ||
| * @returns {boolean} - Returns true if the profile satisfies all requirements, false otherwise. | ||
| */ | ||
| isProfileComplete(profile, profileReqs) { |
There was a problem hiding this comment.
Could you add a test to see what profileReqs looks like?
There was a problem hiding this comment.
It's located here right?: packages/client/test/test.js
There was a problem hiding this comment.
Yes, that's the right spot.
| } else if (expectedValue === true && !actualValue) { | ||
| return false; | ||
| } else { | ||
| console.log(`Invalid path: ${profilePath}`); |
There was a problem hiding this comment.
Should we throw an error here instead of logging it?
There was a problem hiding this comment.
The logic was simplified so that PCs aren't allowed to require random parts of the profile. Now I ignore incorrect input from PCs by returning true (e.g. if they set "dblp": true) so that a person isn't wrongly marked as incomplete.
|
@enrubio there are some conflicts here. |
|
The validation was simplified so that PCs should only require: history, relations, expertise, publications, and active state. Previously they could require any part of the profile, but that includes links which we should not allow. Changes should be equivalent to what's in openreview-py. The min requirements is defined in the domain content and there is no validation for PCs since they can directly modify the group. So this function explicitly checks for fields we will allow. A profile won't be marked as incomplete if PCs try to require DBLP, for example. |
This PR adds a tools function to check a profile against a venue's minimum profile requirements. This is used in the Invite Assignment preprocess.
Changes need to match the openreview-py PR, so before merging this we should make sure there's not more feedback on that PR that needs to be moved here.
Example of what
minRequirementscan look like: