Skip to content
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

fix - add new button on opportunity board company picker doesnt work #8488

Merged
merged 5 commits into from
Dec 17, 2024

Conversation

ehconitin
Copy link
Contributor

@ehconitin ehconitin commented Nov 13, 2024

Closes #8455

opportunitybutton.mp4

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR fixes the "New" button functionality in the opportunity board's company picker by implementing company creation and improving data fetching.

  • Added company creation flow in RecordBoardColumnNewOpportunity.tsx using createCompanyOpportunityAndOpenRightDrawer function
  • Implemented right drawer opening after company creation with proper state management via viewableRecordIdState
  • Added cache-and-network fetch policy in useFilteredSearchEntityQuery.ts to ensure immediate visibility of newly created companies
  • Added error handling with isDefined checks for company creation responses

2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -60,6 +60,7 @@ export const useFilteredSearchEntityQuery = ({
filter: notFilter,
limit: limit ?? DEFAULT_SEARCH_REQUEST_LIMIT,
searchInput: searchFilter,
fetchPolicy: 'cache-and-network',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this one!
But the newly created entities won't get fetched without this.

Copy link
Contributor

@lucasbordeau lucasbordeau Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work out of the box, we shouldn't need to add this but maybe the optimistic effect isn't working properly for this request. It it's too time consuming, let's create an issue to solve that.

@FelixMalfait
Copy link
Member

Is there an issue linked to this? Thanks!

@ehconitin
Copy link
Contributor Author

It seems like the add new button is itself removed!
@Bonapara was it supposed to be removed or reworked?

@Bonapara
Copy link
Member

@ehconitin what do you mean? Do you have the issue link so I can remember? Thanks!

@ehconitin
Copy link
Contributor Author

@Bonapara #8455
Can you see the add new button? I cant

@Bonapara
Copy link
Member

Bonapara commented Nov 26, 2024

Ohh I see you're right - Here a a few other issues:

image

@lucasbordeau lucasbordeau self-assigned this Nov 26, 2024
@lucasbordeau
Copy link
Contributor

LGTM for the code part, please create an issue for the optimistic rendering if you can't solve it easily in this PR.

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this one, I'm doing another pass separately

@charlesBochet charlesBochet enabled auto-merge (squash) December 17, 2024 15:44
@charlesBochet charlesBochet merged commit edff53f into twentyhq:main Dec 17, 2024
14 of 17 checks passed
Copy link

Thanks @ehconitin for your contribution!
This marks your 73rd PR on the repo. You're top 1% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

@ehconitin ehconitin deleted the create-company-button branch December 28, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Relation picker no result and new record improvements
5 participants