-
Notifications
You must be signed in to change notification settings - Fork 16
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 Database From RocketChat For Properties Includes Configuration #15
[Feat] Create Notion Database From RocketChat For Properties Includes Configuration #15
Conversation
0d5a997
to
97137a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nabhag8848 A few comments to improve code maintainability.
export enum Number { | ||
NUMBER = "number", | ||
NUMBER_WITH_COMMAS = "number_with_commas", | ||
PERCENT = "percent", | ||
DOLLAR = "dollar", | ||
CANADIAN_DOLLAR = "canadian_dollar", | ||
EURO = "euro", | ||
POUND = "pound", | ||
YEN = "yen", | ||
RUBLE = "ruble", | ||
RUPEE = "rupee", | ||
WON = "won", | ||
YUAN = "yuan", | ||
REAL = "real", | ||
LIRA = "lira", | ||
RUPIAH = "rupiah", | ||
FRANC = "franc", | ||
HONG_KONG_DOLLAR = "hong_kong_dollar", | ||
NEW_ZEALAND_DOLLAR = "new_zealand_dollar", | ||
KRONA = "krona", | ||
NORWEGIAN_KRONE = "norwegian_krone", | ||
MEXICAN_PESO = "mexican_peso", | ||
RAND = "rand", | ||
NEW_TAIWAN_DOLLAR = "new_taiwan_dollar", | ||
DANISH_KRONE = "danish_krone", | ||
ZLOTY = "zloty", | ||
BAHT = "baht", | ||
FORINT = "forint", | ||
KORUNA = "koruna", | ||
SHEKEL = "shekel", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nabhag8848 It may be hard to maintain these if there are changes in notion itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, @samad-yar-khan pretty hard. I just check out the changelog and there are changes to it on 3 different releases before.
- Between April - May-2023
- Between Oct-Nov-2022
- In July 2021
Notion doesn't provide an endpoint to retrieve the available format of Number, format/available value for Notion types having configuration.
Is there any other way we can improve this and make it more managable? i think in this case enum would be more managable as, if there are changes we just need to add the change in enum folder.
const dispatchActionConfig = dispatchActionPropertyType | ||
? DatabaseModal.PROPERTY_TYPE_SELECT_ACTION | ||
: dispatchActionPropertyName | ||
? DatabaseModal.PROPERTY_NAME_ACTION | ||
: dispatchActionSelectOptionName | ||
? DatabaseModal.SELECT_PROPERTY_OPTION_NAME | ||
: null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nabhag8848 Can we make this more readable ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, done with this one.
src/lib/NotionSDK.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untitled pageName was causing an UnHandledPromiseRejection in Server which i had fixed later, but as we are making changes i did fix this here.
} | ||
|
||
export enum SubCommandParam { | ||
DATABASE = "database", | ||
DATABASE = "db", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the sub command param to db
which was discussed previously.
DatabaseModal.PROPERTY_TYPE_SELECT_ACTION | ||
); | ||
const propertyTypeSelected = | ||
DatabaseModal.PROPERTY_TYPE_SELECT_ACTION.toString(); | ||
const isPropertyTypeDispatchAction = | ||
actionId.startsWith(propertyTypeSelected); | ||
|
||
// Property Name Character Entered Action | ||
const dispatchActionPropertyName = | ||
actionId.startsWith(DatabaseModal.PROPERTY_NAME_ACTION) || | ||
actionId.startsWith(DatabaseModal.TITLE_PROPERTY_ACTION); | ||
const propertyNameEntered = | ||
DatabaseModal.PROPERTY_NAME_ACTION.toString(); | ||
|
||
const titlePropertyNameEntered = | ||
DatabaseModal.TITLE_PROPERTY_ACTION.toString(); | ||
|
||
const isPropertyNameDispatchAction = | ||
actionId.startsWith(propertyNameEntered) || | ||
actionId.startsWith(titlePropertyNameEntered); | ||
|
||
// Property Type Select Option Name Action | ||
const dispatchActionSelectOptionName = blockId.startsWith( | ||
DatabaseModal.PROPERTY_TYPE_SELECT_BLOCK | ||
|
||
const SelectPropertyOptionNameAction = | ||
DatabaseModal.PROPERTY_TYPE_SELECT_BLOCK.toString(); | ||
|
||
const SelectPropertyOptionNameEntered = | ||
DatabaseModal.SELECT_PROPERTY_OPTION_NAME.toString(); | ||
|
||
const isSelectOptionNameDispatchAction = blockId.startsWith( | ||
SelectPropertyOptionNameAction | ||
); | ||
|
||
const dispatchActionConfig = dispatchActionPropertyType | ||
? DatabaseModal.PROPERTY_TYPE_SELECT_ACTION | ||
: dispatchActionPropertyName | ||
? DatabaseModal.PROPERTY_NAME_ACTION | ||
: dispatchActionSelectOptionName | ||
? DatabaseModal.SELECT_PROPERTY_OPTION_NAME | ||
const typeOfActionOccurred = isPropertyTypeDispatchAction | ||
? propertyTypeSelected | ||
: isPropertyNameDispatchAction | ||
? propertyNameEntered | ||
: isSelectOptionNameDispatchAction | ||
? SelectPropertyOptionNameEntered | ||
: null; | ||
|
||
switch (dispatchActionConfig) { | ||
switch (typeOfActionOccurred) { | ||
case DatabaseModal.PROPERTY_TYPE_SELECT_ACTION: { | ||
this.handlePropertyTypeSelectAction( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed Variable Naming
Issue(s)
Acceptance Criteria fulfillment
Proposed changes (including videos or screenshots)
Further comments