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

[Feat] Create Notion Record From RC #22

Conversation

Nabhag8848
Copy link
Collaborator

@Nabhag8848 Nabhag8848 commented Aug 2, 2023

Issue(s)

Acceptance Criteria fulfillment

Proposed changes (including videos or screenshots)

Updating_RC

Further comments

@Nabhag8848 Nabhag8848 force-pushed the feat/create-record-with-multiselect-prop branch from f632af6 to e089eec Compare August 21, 2023 16:17
Copy link
Collaborator

@samad-yar-khan samad-yar-khan left a comment

Choose a reason for hiding this comment

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

@Nabhag8848 left some comments

Comment on lines -16 to -18
ADD_PROPERTY_ACTION = "add-property-create-page-or-record-action-id",
ADD_PROPERTY_BLOCK = "add-property-create-page-or-record-action-id",
ADD_PROPERTY_BUTTON_TEXT = "Add Property",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Nabhag8848 Are these not needed anymore ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope! these are not needed as we had transition to new implementation. before it was having add and remove but now we just get all the components as user selects notion table.

);
];

const missingObject = {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Nabhag8848 empty constant missing object ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to missingPropObject to make it more meaningful. its being used for view errors when user misses the required property to input.
Related to #22 (comment)

Comment on lines 288 to 290
missingObject[
NotionPageOrRecord.TITLE_ACTION
] = `Please Provide ${titleViewError}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Nabhag8848 you are updating a constant

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have used assignment immutablity in this case. const missingObject = { } is making sure reference to that object doesn't change.
Just shared the below example showing what i mean.
we can see object1 has also been changed due to change in missingObject as we have a constant reference when changing its changing the existing object not the referrence.

Screenshot from 2023-08-22 11-34-29

@samad-yar-khan samad-yar-khan merged commit 7fe1082 into RocketChat:main Aug 30, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] Create Notion Page and Record
2 participants