telegram-bot-api icon indicating copy to clipboard operation
telegram-bot-api copied to clipboard

Improve docs for EncryptedPassportElement

Open gagarski opened this issue 1 year ago • 8 comments

For example docs for phone_number field say:

Optional. User's verified phone number, available only for “phone_number” type

It seems to be mandatory when the type is phone_number. This is just an example and the issue applies to all the fields.

This fact can be better highlighted in the docs by either:

  • splitting EncryptedPassportElement into subtypes (will introduce duplicates)
  • specifying it explicitly by text, like "mandatory for types ..., optional for types ..."
  • Refering Telegram Passport Docs one more time: this table properly describes fields by types and it's probably the same for bot API.

gagarski avatar Feb 13 '24 20:02 gagarski

Bot API often uses "union" types like Update, KeyboardButton, or InlineKeyboardButton. Sometimes they are split to separate types in the documentation, like it was done with ChatMember, but the splitting can cause documentation duplication. In the case of ChatMember only the field "user" was duplicated in all types, but for EncryptedPassportElement this will cause a very huge duplication, which makes this an undesirable option.

In the current documentation the phrase "Available for ..." means exactly "Present for the types and absent for other types". I don't like the idea to use the word "mandatory" for a class that can be only received from Bot API and never needs to be provided by the bot. Therefore, from my point of view this is already specified in the documentation by text. I can only propose to replace "Available for" with "Available only for". Will it clarify the documentation from your viewpoint?

levlam avatar Feb 26 '24 14:02 levlam

Let me show an example. Let's consider driver_license document and refer to the Telegram Passport docs, specifically this table. We can see three mandatory fields: data, front_side, reverse_side and two optional fields: selfie and translation.

Now given that EncryptedPassportElement is related to the Fields described there, I believe, as a bot developer I can rely on the following (can I?): if the type is driver_license then data, front_side and reverse_side will definitely be present and there optionally can be selfie and translation.

Current type description won't make you believe that (specifically, "will definitely be present" vs "there optionally can be"), while the mentioned table in Telegram Passport docs actually can. That's why I mentioned an option of explicitly pointing to the table.

I don't like the idea to use the word "mandatory" for a class that can be only received from Bot API and never needs to be provided by the bot.

Missed the "only received" part and agree that mandatory is not a right word in that case.

"Available for" with "Available only for" As I mentioned, it's not "Available for"/"Available only for" (in context of this table it's pretty much the same), but "Present" vs "May be present".

gagarski avatar Feb 27 '24 19:02 gagarski

We can see three mandatory fields: data, front_side, reverse_side and two optional fields: selfie and translation.

This is documented as "Available if requested" for "translation". It is not optional or always present, It is present if it was requested. "selfie" should have had the same wording.

levlam avatar Feb 27 '24 20:02 levlam

It is not optional or always present, It is present if it was requested.

But driver_license cannot appear without reverse_side requested, for example, right?

gagarski avatar Feb 27 '24 20:02 gagarski

"selfie" should have had the same wording

Yep, that would make sense.

gagarski avatar Feb 27 '24 20:02 gagarski

But driver_license cannot appear without reverse_side requested, for example, right?

Yes.

levlam avatar Feb 27 '24 21:02 levlam

This is documented as "Available if requested" for "translation". It is not optional or always present, It is present if it was requested. "selfie" should have had the same wording.

Maybe this can be clarified by the following sentence:

The set of the fields available is defined by the fields requested by Telegram Passport enabled application. The set of fields that can be requested varies based on document type.

gagarski avatar Feb 27 '24 22:02 gagarski

Now documentation of fields of the class is more consistent.

levlam avatar Feb 29 '24 21:02 levlam