Fix array edit option #8600
Closed
Fix array edit option #8600
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
PR Summary
Added support for editing Array type fields in the MultiItemFieldInput component, fixing an error that occurred when attempting to edit Array type values.
- Added
FieldMetadataType.Arraycase inhandleEditButtonClickmethod in/packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/MultiItemFieldInput.tsx - Array type values are now handled similarly to Email types, treating them as strings
- Consider adding type safety checks when casting Array values to strings
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
ehconitin
reviewed
Nov 21, 2024
| @@ -110,6 +110,7 @@ export const MultiItemFieldInput = <T,>({ | |||
| item = items[index] as PhoneRecord; | |||
| setInputValue(item.countryCode + item.number); | |||
| break; | |||
| case FieldMetadataType.Array: | |||
There was a problem hiding this comment.
Why is there no break condition for this? is the break condition for emails and arrays the same?
There was a problem hiding this comment.
Yes, the break statement for Array and Email are the same. The way the Array is being handled in the backend is just like the Email. So I found it better to use the same block as the Email one
|
Hey @ehconitin, could you review this please? |
ehconitin
reviewed
Nov 22, 2024
|
There is an explicit break for the Array though. If it applies for Email, won't it also apply for Array |
…wentyhq#8588) Fixes: twentyhq#8487 twentyhq#5027 1. Summary The purpose of these changes is to elevate the dev/user experience when the initial config load call fails for whatever reason by displaying a fallback component. 2. Solution I ended up making more changes than I initially planned. I had to update the order of the contexts a bit because `GenericErrorFallback` is dependent on `AppThemeProvider` for styling and `AppThemeProvider` is dependent on `ObjectMetadataItemsProvider` for [`useObjectMetadataItem`](https://github.com/khuddite/twenty/blob/ae2f193d68c6168e4c8323297d58f6dbc1de9fdf/packages/twenty-front/src/modules/object-metadata/hooks/useObjectMetadataItem.ts#L22) hook (`AppThemeProvider` -> `useColorScheme` -> `useUpdateOneRecord` -> `useObjectMetadataItem`). I had to create a wrapper component for `AppThemeProvider` and stylize it in a way that it looks responsive on both mobile and desktop devices. Finally, I had to introduce the `isErrored` flag to differentiate the loading and error states. There are some improvements we can make later - - Display a loading state for the initial config load - Implement a refetch logic for the initial config loading failure 3. Recording https://github.com/user-attachments/assets/c2f43573-8006-4118-8e18-8576099d78fd https://github.com/user-attachments/assets/9c5853d3-539b-4880-aa38-c416c3e13594 --------- Co-authored-by: Félix Malfait <[email protected]>
The 1-click install did not ask questions anymore, let's try removing the S flag
Second attempt to fix the interactive install script, by downloading the file first and then executing it
grammatical issue fixed
## Before   ## After  
## Context We added a check of the field type in [useTextFieldDisplay.ts](https://github.com/twentyhq/twenty/compare/c--fix-rich-text-display?expand=1#diff-8e53a2496b9faa67ac9aad3f7016efed19147557904da1bc44e895688513330d) however this hook is also used for RICH_TEXT fields so it fails. I'm removing this check since I don't think this is necessary, the caller should already know its a TEXT or RICH_TEXT
- Created a dropdown inside a dropdown for the `ObjectFilterDropdownOperandDropdown` so the operand can be opened over the selection with an offset - Refactored dropdown component and introduced `DropdownUnmountEffect` to close the dropdown when the component unmounts - Removed old logic Before: <img width="216" alt="Capture d’écran 2024-11-22 à 14 03 58" src="https://github.com/user-attachments/assets/3c1bba03-af03-4993-a070-f009b8dc876f"> After: <img width="222" alt="Capture d’écran 2024-11-22 à 14 03 40" src="https://github.com/user-attachments/assets/a8a784b4-8672-4b02-bb21-e4a749682f2e">
Following previous release, a suggestion was noticed by @martmull . Here is a suggested fix. Before :  After : <img width="611" alt="Screenshot 2024-11-22 at 14 56 07" src="https://github.com/user-attachments/assets/67298633-4513-41a4-90aa-5e2366d3cd88"> --------- Co-authored-by: guillim <[email protected]>
**TLDR** Added Billing Entitlement table, based on stripe customer.ActiveEntitlements webhook event. In this table it has a key value pair with each key being the stripe feature lookup key and the value a boolean. We use this table in order to see if SSO or other feaures are enabled by workspace. **In order to test: twenty-server** Billing: - Set IS_BILLING_ENABLED to true - Add your BILLING_STRIPE_SECRET and BILLING_STRIPE_API_KEY - Add your BILLING_STRIPE_BASE_PLAN_PRODUCT_ID (use the one in testMode > Base Plan) Auth: - Set AUTH_SSO_ENABLED to true - Set your ACCESS_TOKEN_SECRET, LOGIN_TOKEN_SECRET, REFRESH_TOKEN_SECRET and FILE_TOKEN_SECRET - Set IS_SSO_ENABLED feature flag to true Stripe Webhook: - Authenticate with your account in the stripe CLI - Run the command: stripe listen --forward-to http://localhost:3000/billing/webhooks Migration: - npx nx typeorm -- migration:run -d src/database/typeorm/core/core.datasource.ts **In order to test: twenty site** - Buy a subscription (you can use the card 4242...42 with expiration date later in the future) - Go to SSO and create an OICD subscription - Change the value in the entitlement table in order to put it in false - An error should occur saying that the current workspace has no entitlement **Considerations** The data from the Entitlement table is updated based on the stripe webhook responses, and we use the customerActiveEntitlemet response to update the info on the table, however this event doesnt have the metadata containing the workspaceId. Because we cannot control at wich order the webhook send events, we force a server error if the entitlements are updated before the BillingSubscription. Stripe resends the event based on a exponential backoff (for more info see https://docs.stripe.com/webhooks#retries ) because we are in test mode Stripe retries three times over a few hours. So if the BillingEntitlement is not updated it is completely normal and it will be updated when stripe resends the event. --------- Co-authored-by: Félix Malfait <[email protected]>
twentyhq#8611) …RKSPACE_ID The Pull Request modifies seed data by replacing SEED_TWENTY_WORKSPACE_ID with SEED_ACME_WORKSPACE_ID across several files. - Updated SEED_TWENTY_WORKSPACE_ID to SEED_ACME_WORKSPACE_ID. - Modified relevant import paths and seeds involving workspace data. - Changes affect seeding processes for workspace members, user workspaces, and general workspace details.
- Use TextInput in header title - add onTitleChange prop - rename field name instead of label To fix : - padding right on title comes from current TextInput component. It needs to be refactored https://github.com/user-attachments/assets/535cd6d3-866b-4a61-9c5d-cdbe7710396a
|
To clarify - currently, there isn't a separate break for Array. The code falls through from the Array case to the Email case, sharing a single break statement. While they handle the input the same way now, having separate cases with explicit breaks would make the code's intention clearer and easier to maintain. What if in the future email break conditions change??? Your code will break in that situation :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #8578
There was no case to handle a
FieldMetadataType.Arrayand thus an error was thrown every time the edit button was clicked on anArraytype.It has been added now.