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

Question: Should ChatInitSettings options have a exported interface #855

Closed
bingenito opened this issue Nov 11, 2022 · 2 comments · Fixed by #875 or #994
Closed

Question: Should ChatInitSettings options have a exported interface #855

bingenito opened this issue Nov 11, 2022 · 2 comments · Fixed by #875 or #994
Assignees
Labels
bug Something isn't working Context Data & Intents Contexts & Intents Discussion Group context-data FDC3 Context Data Working Group
Milestone

Comments

@bingenito
Copy link
Member

Question Area

[x] Context Data

Question

The documentation for fdc3.chatinitsettings has 5 documented optional boolean properties but the exported interface for ChatInitSettings defines options as any. Should we add the documented properties to an exported type and change options to be that type vs any?

@mistryvinay mistryvinay added context-data FDC3 Context Data Working Group Context Data & Intents Contexts & Intents Discussion Group enhancement New feature or request labels Nov 21, 2022
@kriswest
Copy link
Contributor

kriswest commented Dec 5, 2022

@bingenito good spot - I believe this portion of the schema is incorrectly defined:

"options": {
"groupRecipients": "boolean",
"public": "boolean",
"allowHistoryBrowsing": "boolean",
"allowMessageCopy": "boolean",
"allowAddUser": "boolean"
}

The schema should be closer to this:

{
    "$schema": "http://json-schema.org/draft-07/schema#",
    "$id": "https://fdc3.finos.org/schemas/next/chatInitSettings.schema.json",
    "type": "object",
    "title": "ChatInitSettings",
    "properties": {
        "type": {
            "const": "fdc3.chat.initSettings"
        },
        "chatName": {
            "type": "string"
        },
        "members": {
            "$ref": "contactList.schema.json#"
        },
        "initMessage": {
            "type": "string"
        },
        "options": {
            "type": "object",
            "properties": {
                "groupRecipients": {"type": "boolean"},
                "public": {"type": "boolean"},
                "allowHistoryBrowsing":  {"type": "boolean"},
                "allowMessageCopy":  {"type": "boolean"},
                "allowAddUser":  {"type": "boolean"}
            }
        }
    },
    "required": [
        "type"
    ]
}

Which does generate the properties.

However, I also note that we have field with the name public. This is a reserved word in Javascript/typescript and can't be used as a property name (see https://www.w3schools.com/js/js_reserved.asp and microsoft/TypeScript#2536). Hence we should really rename that field also.

@mistryvinay could the Symphony team look at fixing these two issues?

@symphony-jean-michael
Copy link
Contributor

Hi @kriswest
Thank you for your investigation.
I'v created a new PR: #875
Could you please review it ?
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Context Data & Intents Contexts & Intents Discussion Group context-data FDC3 Context Data Working Group
Projects
None yet
4 participants