-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: Add new interface PlatformProject + cleanup ExtendedProject unwanted fields #2903
Conversation
Your free trial has expired. To continue using Ellipsis, sign up at https://app.ellipsis.dev for $20/developer/month. If you have any questions, reach us at [email protected] |
PR title must start with "fix:" or "feat:" or "test:" |
|
ap1_email: string; | ||
ap1_full_name: string; | ||
ap2_email: string; | ||
ap2_full_name: string; | ||
project_active: boolean; | ||
project_approver1_id: string; | ||
project_approver2_id: string; |
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.
can we check the usage of this modal, it should not break the previous code.
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.
We are no longer using these keys.
You can checkout discussion regarding this here.
https://fylein.slack.com/archives/C06M78V7JKD/p1713870369981659?thread_ts=1713850203.385719&cid=C06M78V7JKD
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.
Instead of this can we make these keys optional or can create a new modal for your use case, removing keys from the existing modal changes the previous model which is not correct imo because the previous modal was created about v2 APIs response that is still correct.
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.
Ok. I will confirm this once.
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.
Minor comment
There are multiple conflicts I will create a new separate PR for this. |
Description
as per the discussion in thread: https://fylein.slack.com/archives/C06M78V7JKD/p1713850203385719
Removed the above fields from
ExtendedProjects
PlatformProject
interfaceClickup
Please add link here