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

AIP-16: Quote/reply messages #48

Open
adamant-al opened this issue Feb 18, 2022 · 9 comments
Open

AIP-16: Quote/reply messages #48

adamant-al opened this issue Feb 18, 2022 · 9 comments

Comments

@adamant-al
Copy link
Member

adamant-al commented Feb 18, 2022

Discussing https://github.com/Adamant-im/AIPs/blob/master/AIPS/aip-16.md

@adamant-al
Copy link
Member Author

Not sure, if allow reply_message to include nested JSON with other Rich message. One side is it will be convenient to send crypto in reply, other is client app implementation will be more complex.

@MerdyNumber1
Copy link
Member

Good idea, new functionality can be added to bots with replies

@gost1k337
Copy link
Member

nice feature, agree with @MerdyNumber1

@martiliones
Copy link
Collaborator

martiliones commented Feb 19, 2022

@adamant-al I think including nested JSON with another "rich" message is worth it. It will be easier to change UX/UI or add new functionality in the future. For example: adding an ETH icon for the reply message.

Also the structure of reply_message that contains regular message can be changed to be more readable for checking.

if (message.reply_message) {
  /**
   * {
   *  "replyto_id": "7452709338464950789",
   *  "reply_message": "I've got it. Will be there in time."
   * }
   */
  if (typeof message.reply_message === 'string') { // current version
    // ...
  } else if (message.reply_message.type.includes('transaction')) {
    // ...
  }

  /**
   * {
   *  "replyto_id": "7452709338464950789",
   *  "reply_message": {
   *    type: "text",
   *    text: "I've got it. Will be there in time."
   *  }
   * }
   */
  const { type: replyMessageType } = message.reply_message

  // replyMessageType can be used in switch case statements
  if (replyMessageType === 'text') {
    // ...
  } else if (replyMessageType.includes('transaction')) {
    // ...
  }
}

/**
 * {
 *  "replyto_id": "7452709338464950789",
 *  "reply_text": "I've got it. Will be there in time."
 * }
 */
if (message.reply_text) { // separate logic with rich objects
  // ...
} else {
  const { type: replyMessageType } = message.reply_message;

  if (replyMessageType.includes('transaction')) {
    // ...
  }
}

@adamant-al
Copy link
Member Author

I've added a note in the AIP: field transaction.asset.chat.message must contain encrypted stringified JSON.
@martiliones I don't think we should have both reply_message and reply_text . And it seems we anyway should check (typeof message.reply_message === 'string'), as even with additional type and text fields, third-party apps can store anything.
@MerdyNumber1 What do you think?

@MerdyNumber1
Copy link
Member

Don't you think that it's better to have only one field reply_id, which will point that it's reply.
The type field move to the root, and make payload field where we'll store our data depending on the type field.
Summing up, in the root of JSON we'll store all metadata, field payload will contain data structure depending on the type field.

I've added a note in the AIP: field transaction.asset.chat.message must contain encrypted stringified JSON. @martiliones I don't think we should have both reply_message and reply_text . And it seems we anyway should check (typeof message.reply_message === 'string'), as even with additional type and text fields, third-party apps can store anything. @MerdyNumber1 What do you think?

@adamant-al
Copy link
Member Author

adamant-al commented Feb 20, 2022

It seems you mean transaction.asset.chat.message to be:

{
  "replyto_id": "7452709338464950789",
  "type": "text",
  "payload": "I've got it. Will be there in time."
}

{
  "replyto_id": "7452709338464950789",
  "type": "transfer",
  "payload": {
    "type": "ETH_transaction",
    "amount": "0.002",
    "comments": "I like to send it, send it",
    "hash": "0xfa46d2b3c99878f1f9863fcbdb0bc27d220d7065c6528543cbb83ced84487deb"
  },
}

You'll have a code then switch (type), case 'text':

if (payload.includes('in time'))

You expect payload to be a string, be what if actual data will be:

{
  "replyto_id": "7452709338464950789",
  "type": "text",
  "payload": { }
}

or

{
  "replyto_id": "7452709338464950789",
  "type": "text"
}

So the field type doesn't say payload will be of this type.

@MerdyNumber1
Copy link
Member

It seems you mean transaction.asset.chat.message to be:

{
  "replyto_id": "7452709338464950789",
  "type": "text",
  "payload": "I've got it. Will be there in time."
}

{
  "replyto_id": "7452709338464950789",
  "type": "transfer",
  "payload": {
    "type": "ETH_transaction",
    "amount": "0.002",
    "comments": "I like to send it, send it",
    "hash": "0xfa46d2b3c99878f1f9863fcbdb0bc27d220d7065c6528543cbb83ced84487deb"
  },
}

You'll have a code then switch (type), case 'text':

if (payload.includes('in time'))

You expect payload to be a string, be what if actual data will be:

{
  "replyto_id": "7452709338464950789",
  "type": "text",
  "payload": { }
}

or

{
  "replyto_id": "7452709338464950789",
  "type": "text"
}

So the field type doesn't say payload will be of this type.

But exactly for this we'll validate the data, for example using validate.js
Anyway, we gotta do the same with reply_message, ain't we?

@adamant-al
Copy link
Member Author

Accepted as

{
  "replyto_id": "7452709338464950789",
  "reply_message": "I've got it. Will be there in time."
}

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

No branches or pull requests

4 participants