Fix store invite #113
Fix store invite #113
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -55,18 +55,34 @@ | |||
| ) | |||
| } | |||
|
|
|||
| // TODO : modify this if necessary | |||
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
| @@ -77,6 +77,9 @@ | |||
| }) | |||
| return | |||
| } | |||
|
|
|||
| // TODO : hook for any pending invite and call the onbind api : https://spec.matrix.org/v1.11/client-server-api/#room-aliases | |||
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
| }) | ||
| .catch((err) => { | ||
| /* istanbul ignore next */ | ||
| this.logger.error('Failed to insert token', err) |
There was a problem hiding this comment.
Missing "reject": a promise must always call resolve or reject
| } | ||
| }) | ||
| .catch((e) => { | ||
| reject(e) |
| .set('Authorization', `Bearer ${validToken}`) | ||
| .set('Accept', 'application/json') | ||
| .send({ | ||
| address: '+33612345678', |
There was a problem hiding this comment.
"address" looks a bad name here
There was a problem hiding this comment.
The name has to be the same for the different media. But the spec only supports mail so this is named address for the moment.
There was a problem hiding this comment.
Looking at others API, phone is used to designate phone when type is "pĥone"
There was a problem hiding this comment.
I think the most common use is "country" (The two-letter uppercase ISO-3166-1 alpha-2 country code that the number in phone_number should be parsed as if it were dialled from) + "phone_number"
There was a problem hiding this comment.
No, there is an encoding convention for phone in Matrix spec: international number without + or spaces
| .replace(/__room_name__/g, room_name) | ||
| .replace(/__room_avatar__/g, room_avatar) | ||
| .replace(/__room_type__/g, room_type) | ||
| .replace(/__link__/g, inviteLink('matrix.to', sender_user_id, room_alias)) |
There was a problem hiding this comment.
matrix.to must be a config parameter, not a fixed string
| @@ -134,6 +173,7 @@ const StoreInvit = <T extends string = never>( | |||
| return (req, res) => { | |||
| idServer.authenticate(req, res, (_data, _id) => { | |||
| jsonContent(req, res, idServer.logger, (obj) => { | |||
| // eslint-disable-next-line @typescript-eslint/no-misused-promises | |||
| validateParameters(res, schema, obj, idServer.logger, async (obj) => { | |||
| const _address = (obj as storeInvitationArgs).address | |||
There was a problem hiding this comment.
I think we shouldn't use the same fields name address for a phone number
There was a problem hiding this comment.
Should we create a different endpoint for phone maybe ?
There was a problem hiding this comment.
Can also be a different schema. If type is email, address is required, else phone is required
There was a problem hiding this comment.
But then the token I store will have two fields (phone and address) and one of them will be empty. Is it ok ?
There was a problem hiding this comment.
Are we talking about database schema or API field names ?
There was a problem hiding this comment.
I am now talking about how I store the invitation in the db, should I do the same thing as for the API field names, ie add a new column phone which will sometimes be empty, or keep one column for addresses which will store email as well as phone numbers ?
There was a problem hiding this comment.
I was talking about API fieldnames. For DB, do what is useful for you
There was a problem hiding this comment.
In the synapses db, the convention is to use a medium and an address, address being either an email address or a phone number. I will stick to this usage
|
Please add a link to your MR in related issues when exist (#62) here |
…red in alphabetic order)
h1ppox99
force-pushed
the
fix-store-invite
branch
from
b954aa6
to
0724eb7
Compare
| }) | ||
| break | ||
| case 'msisdn': | ||
| // TODO implement smsSender |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
…onf + update of all confs necessary
|
Remember to drop "draft" mode for review |
No description provided.