fix: remove usage of probability field #5877

siiddhantt · 2024-06-15T07:17:21Z

fixes [Timebox] Remove "Probability" Field from Opportunities #5735

greptile-apps

PR Summary

Removed 'probability' field from filter definitions and related utilities
Eliminated 'probability' field from GraphQL queries and mock data
Updated type definitions and validation logic to exclude 'probability' field
Removed 'probability' field from database seed data and workspace migrations
Adjusted test cases to no longer include 'probability' field

lucasbordeau · 2024-06-17T09:00:02Z

@Bonapara Should we remove the entire probability type from Twenty or just this particular field on opportunity ?

Bonapara · 2024-06-17T10:07:03Z

@lucasbordeau we should keep the Rating field type but remove this probability field from the opportunity object.

greptile-apps

PR Summary

(updates since last review)

Commented out FieldMetadataType.Probability in SettingsObjectNewFieldStep2.tsx
Added PROBABILITY field type handling as number in computeInputFields.ts
Added PROBABILITY to FieldMetadataType enum in data.types.ts

siiddhantt · 2024-06-17T18:17:45Z

hi!
made changes so the probability field is only removed from the front and server in opportunity, and also the rating field is already present there

greptile-apps

PR Summary

(updates since last review)

Removed usage of the probability field
Deleted imports and logic for isFieldRating and isFieldRatingValue
Updated field validation and persistence logic

lucasbordeau · 2024-06-21T12:49:43Z

@siiddhantt Hi, please could you keep everything related to this field type, we don't want to remove the probability field type, which is a backend metadata feature level, we just want to remove a particular standard field which is on the standard seed data level.

I'll explain in more details what we want here :

Keep everything related to field types rating and probability
Remove this field from the seeds of standard field we have in the code, so it doesn't get created on new Twenty installations
Propose a way to migrate properly this field if it already exists in a workspace to a custom field, so it will be the same for users who use this field, except this field will turn into a custom field.

For the migration, you might want to test it on your local install, and we will test it on our side before running it for all workspaces.

This is a bit complex, don't hesitate to ask for more details and you can reach us on our Discord also.

charlesBochet · 2024-06-22T11:02:26Z

@siiddhantt @lucasbordeau Taking a look at this one:

As Lucas said there are different aspects in this work:

Field types: we should keep RATING type and remove PROBABILITY type which is not used actually.
Deprecate Field probability on opportunity: we actually don't want to remove it (as users could have data stored in it) but deprecate it. This is quite complex and @ijreilly will be leading the effort here next week. We can remove seeding logic though

Could you restrict the scope of this PR to removing PROBABILITY field type + stop seeding opportunity probability field?
I'm adding specific comments on the PR to guide you

charlesBochet · 2024-06-22T11:03:23Z

...wenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemsAsFilterDefinitions.ts

@@ -29,11 +29,6 @@ export const formatFieldMetadataItemsAsFilterDefinitions = ({
      return acc;
    }

-    // Todo: remove once Rating fieldtype is implemented


this is fine as we will deprecate the field, let's keep it

charlesBochet · 2024-06-22T11:03:35Z

packages/twenty-front/src/modules/object-metadata/utils/getObjectMetadataItemsMock.ts

@@ -466,23 +466,6 @@ export const getObjectMetadataItemsMock = () => {
          fromRelationMetadata: null,
          toRelationMetadata: null,
        },
-        {


packages/twenty-front/src/generated/graphql.tsx

packages/twenty-front/src/generated-metadata/graphql.ts

packages/twenty-front/src/modules/object-record/hooks/__mocks__/useFindManyRecords.ts

charlesBochet · 2024-06-22T11:07:09Z

packages/twenty-front/src/testing/mock-data/generated/standard-metadata-query-result.ts

@@ -5517,29 +5517,6 @@ export const mockedStandardObjectMetadataQueryResult: ObjectMetadataItemsQuery =
 								"toRelationMetadata": null
 							}
 						},
-						{


charlesBochet · 2024-06-22T11:07:09Z

...y-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx

@@ -166,7 +166,7 @@ export const SettingsObjectNewFieldStep2 = () => {
      // FieldMetadataType.FullName,
      // FieldMetadataType.Link,
      FieldMetadataType.Numeric,
-      FieldMetadataType.Probability,
+      // FieldMetadataType.Probability,


charlesBochet · 2024-06-22T11:07:11Z

packages/twenty-front/src/testing/mock-data/view-fields.ts

@@ -142,14 +142,6 @@ export const mockedViewFieldsData = [
    isVisible: true,
    size: 180,
  },
-  {


charlesBochet · 2024-06-22T11:07:24Z

packages/twenty-server/src/database/typeorm-seeds/workspace/opportunities.ts

@@ -25,7 +25,6 @@ export const seedOpportunity = async (
      'amountAmountMicros',
      'amountCurrencyCode',
      'closeDate',
-      'probability',


let's keep it for now

charlesBochet · 2024-06-22T11:07:48Z

packages/twenty-server/src/engine/api/__mocks__/object-metadata-item.mock.ts

@@ -145,13 +145,6 @@ const fieldNumericMock = {
  defaultValue: null,
 };

-const fieldProbabilityMock = {


greptile-apps

PR Summary

(updates since last review)

Exclude 'probability' field from filter definitions
Add support for 'Rating' field type in usePersistField
Reintroduce 'probability' field in opportunity seed data with default value 0.5

_{3 file(s) reviewed, no comment(s)}

FelixMalfait · 2024-07-11T06:51:53Z

cc @ijreilly could you please take over this PR as it's related to yours?

Also fyi #3531 we do want to remove the probability field type so this PR might get us there.

We have the rating field type which is good enough to represent this. Eventually at some point we'll have display options on the rating field type "stars, progress bar, etc."

Thanks @siiddhantt for the contribution!

ijreilly · 2024-07-11T13:01:06Z

Hi @siiddhantt, thank you for your contribution!
In a different PR we have removed the usage of Probability field on object Opportunity. This was part of a recent effort to ease standard field deprecation.
If you are still keen to work on the removal of probability field, I think you can merge main and solve the conflicts, then I think we should be good or close! Otherwise I can take over the work, let me know :)

siiddhantt · 2024-07-11T13:53:04Z

Hi @ijreilly thanks for the update! I appreciate your help on this.
Since you've been working on a related issue, you could take over this. Thanks :)

ijreilly · 2024-07-11T14:40:14Z

Got it, @siiddhantt thanks a lot for your contribution and interest in twenty!

…obability-field

ijreilly · 2024-07-16T08:25:24Z

Closes #6259

github-actions · 2024-07-16T08:25:39Z

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

fix: remove usage of probability field

ce1621d

greptile-apps bot reviewed Jun 15, 2024

View reviewed changes

lucasbordeau added the blocked: tech spec needed label Jun 17, 2024

lucasbordeau added -PR: awaiting author and removed blocked: tech spec needed labels Jun 17, 2024

fix: reverted changes to only remove field in front/server

879be95

greptile-apps bot reviewed Jun 17, 2024

View reviewed changes

fix: lint issues

e23f378

greptile-apps bot reviewed Jun 17, 2024

View reviewed changes

lucasbordeau self-assigned this Jun 21, 2024

charlesBochet reviewed Jun 22, 2024

View reviewed changes

fix: changes according to review

c0fd528

greptile-apps bot reviewed Jun 22, 2024

View reviewed changes

siiddhantt requested a review from charlesBochet June 24, 2024 16:33

FelixMalfait requested a review from ijreilly July 11, 2024 06:52

FelixMalfait assigned ijreilly and unassigned lucasbordeau Jul 11, 2024

charlesBochet mentioned this pull request Jul 11, 2024

Release v0.22 #6217

Closed

ijreilly added 2 commits July 15, 2024 18:29

Merge branch 'main' of github.com:twentyhq/twenty into fix/useless-pr…

57b9064

…obability-field

Remove special formatting for probability field

38dfa8a

ijreilly approved these changes Jul 16, 2024

View reviewed changes

ijreilly merged commit 364caf0 into twentyhq:main Jul 16, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove usage of probability field #5877

fix: remove usage of probability field #5877

siiddhantt commented Jun 15, 2024

greptile-apps bot left a comment

lucasbordeau commented Jun 17, 2024

Bonapara commented Jun 17, 2024

greptile-apps bot left a comment

siiddhantt commented Jun 17, 2024

greptile-apps bot left a comment

lucasbordeau commented Jun 21, 2024

charlesBochet commented Jun 22, 2024

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

greptile-apps bot left a comment

FelixMalfait commented Jul 11, 2024 •

edited

Loading

ijreilly commented Jul 11, 2024

siiddhantt commented Jul 11, 2024

ijreilly commented Jul 11, 2024

ijreilly commented Jul 16, 2024

github-actions bot commented Jul 16, 2024

fix: remove usage of probability field #5877

fix: remove usage of probability field #5877

Conversation

siiddhantt commented Jun 15, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

lucasbordeau commented Jun 17, 2024

Bonapara commented Jun 17, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

siiddhantt commented Jun 17, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

lucasbordeau commented Jun 21, 2024

charlesBochet commented Jun 22, 2024

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

charlesBochet Jun 22, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

FelixMalfait commented Jul 11, 2024 • edited Loading

ijreilly commented Jul 11, 2024

siiddhantt commented Jul 11, 2024

ijreilly commented Jul 11, 2024

ijreilly commented Jul 16, 2024

github-actions bot commented Jul 16, 2024

FelixMalfait commented Jul 11, 2024 •

edited

Loading