-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature/deseng745: Updated project definition to add project type multi-select values. #81
Conversation
jareth-whitney
commented
Nov 22, 2024
- Updated project definition to add project type multi-select values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the swagger definition for this new field may be incomplete and/or incorrect. Please see my comment in the code!
api/swagger/swagger.yaml
Outdated
@@ -2629,6 +2651,14 @@ paths: | |||
description: "The project phase of the file." | |||
required: false | |||
type: string | |||
- name: projectTypes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do Documents need a project type? Isn't this field just meant for projects only?
Just from reading the ticket, it looks like you'd need the projectTypes field in 4 places:
- authenticated (/project) get
- auth'd put
- auth'd post
- public (/public/project) get
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching it, just a misplaced field.
@@ -32,7 +32,8 @@ | |||
proMember : '{{lorem(1, "words")}}', | |||
provElecDist : '{{lorem(1, "words")}}', | |||
shortName : '{{lorem(1, "words")}}', | |||
projectPhase : '{{lorem(1, "words")}}', | |||
projectPhase : '{{lorem(1, "words")}}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the due diligence but we don't need to update this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!