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,
        },
-        {


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

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

@@ -233,7 +233,6 @@ export enum FieldMetadataType {
  Numeric = 'NUMERIC',
  Phone = 'PHONE',
  Position = 'POSITION',
-  Probability = 'PROBABILITY',


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

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

@@ -327,7 +327,6 @@ export enum FieldMetadataType {
  Numeric = 'NUMERIC',
  Phone = 'PHONE',
  Position = 'POSITION',
-  Probability = 'PROBABILITY',


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)}

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

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

charlesBochet Jun 22, 2024

charlesBochet Jun 22, 2024

greptile-apps bot left a comment

fix: remove usage of probability field #5877

Are you sure you want to change the base?

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary