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

Add industry column to company #5167

Closed

Conversation

brendanlaschke
Copy link
Contributor

@brendanlaschke brendanlaschke commented Apr 25, 2024

Two questions:

  • Is it correct we can edit Select options on an otherwise disabled settings page?
Bildschirmfoto 2024-04-25 um 10 14 11
  • Does the field need to be added to the all companies view or another object/file ?
Bildschirmfoto 2024-04-25 um 10 06 17

closes #3048

@charlesBochet
Copy link
Member

These are very good questions. Here is the product vision on field.options (for SELECT and MULTI-SELECT):

  • introduce isCustom on the option
  • introduce isActive on the option
  • introduce standardId on the option

Then, we need to:

  • Settings UI: do not allow to modify/delete isCustom: false options except the position and color
  • Settings UI: allow to disable isCustom: false (we need design for that @Bonapara) + unset records that have this value
  • sync scripts: start syncing isCustom: false options in sync-metadata script
  • remove isActive: false options from SELECT and MULTISELECT FieldInputs

@charlesBochet
Copy link
Member

This can be treated in another PR though (likely in multiple PRs)

@charlesBochet
Copy link
Member

We don't want to add it in the company view for now

@charlesBochet charlesBochet self-assigned this Apr 25, 2024
@charlesBochet
Copy link
Member

@FelixMalfait do you have any though on that?

@Bonapara
Copy link
Member

Bonapara commented Apr 25, 2024

CleanShot 2024-04-25 at 15 47 40

For V1, the Delete option will only be available Custom Options menu. Only display the "Disabled" section when a field has been disabled.

@charlesBochet
Copy link
Member

@FelixMalfait should we merge this PR?

Copy link

@A1exKH A1exKH left a comment

Choose a reason for hiding this comment

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

LGTM.

@FelixMalfait
Copy link
Member

FelixMalfait commented May 13, 2024

Hey I'm sorry @brendanlaschke this was great but I think I will close it for now.
I created this issue in the context of adding multiple fields for automated data enrichment (e.g. Apollo.io). So the "key" (i.e. value not label) isn't the right one to do the matching with their API response. We should re-open this when we have a clear strategy on data enrichment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new field on company: industry
5 participants