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

User only methods #25

Merged
merged 28 commits into from
Jan 2, 2021
Merged

Conversation

spontanurlaub
Copy link
Collaborator

@spontanurlaub spontanurlaub commented Dec 12, 2020

This PR contains a bunch of user only methods.

Plans for the future (Not this PR, they need many new classes):

  • Working with the admin log
  • Working with group/channel stats

The new methods will be documented in a machine readable format, probably openAPI, so we can generate a Swagger documentation. It doesn't make sense to document them in the readme, which becomes too messy.

Progress tracking

  • getInlineQueryResults (sending an inline query to a bot) (A lot of extra code, moved to a future PR)
  • getChats (Normal chat list)
  • getCommonChats (Common chats with an other user or bot) * (There is currently a bug in tdlight which causes this method to fail, hopefully it will be fixed soon.)
  • getInactiveChats (When reaching the limit of available supergroup chats)
  • getNearbyChats (Search chats by location)
  • searchPublicChats (Searches public chats by looking for specified query in their username and title.)
  • votePoll (For voting in polls or quizzes and retracting votes)
  • joinChat (join a chat by username/chat id or invite_link)
  • joinChatByInviteLink (join a chat by invite link)
  • createChat (Create a group, supergroup or channel)
  • addChatMember (add a user to a group or channel)
  • getCallbackQueryAnswer (Interacting with inline Keyboards)
  • sendInlineQueryResultMessage (selecting an inline query to send) (A lot of extra code, moved to a future PR)
  • searchMessages (for searching messages globally)
  • searchChatMessages (for searching messages in a chat)
  • deleteChatHistory (delete Chat History in a private chat, group or channel)
  • reportChat (report a group or a user for spam)
  • documentation

If I miss any important methods, please let me know. I'm also open for better name suggestions or other comments.

@MarcoBuster MarcoBuster added this to In progress in TDLight bot api via automation Dec 12, 2020
@MarcoBuster MarcoBuster added the enhancement New feature or request label Dec 12, 2020
@giuseppeM99
Copy link
Collaborator

about the joinchat method
since this is an abstraction layer i think that it would be better to have the same method both for public and private chats

@spontanurlaub
Copy link
Collaborator Author

@giuseppeM99 Okay. So the joinChat method takes a chat_id parameter which can be a numerical id or the username (like everywhere in the api). So it is possible to join nearby chats by chat_id as well. I could add an invite_link parameter, that is exclusive to the chat_id, but I'm not sure if this is the nicest solution. I'm also not sure what to do if someone enters something like "t.me/tdlightchat" as invite_link, we should probably fail this query cause there are dozens of alias domains for t.me

@andrew-ld
Copy link
Member

andrew-ld commented Dec 12, 2020

@code1mountain

the bot api unifies many methods into one, i would make joinchat be able to handle either chat_id (username / id) or joinlink, so it will have two parameters (chat_id and joinlink) and you must enter at least 1 in the request then:

joinchat?chat_id=xxxx
joinchat?joinlink=xxxx

chat_id must work with username and id

@JosXa
Copy link

JosXa commented Dec 12, 2020

Some naming suggestions:

  • getChats (Normal chat list)

getDialogs? Pyrogram and Telethon both introduced the concept of a Dialog to represent "something in your chats list". I think it makes sense, but up for discussion. It doesn't solve the issue that a channel technically isn't a "chat" though, because "dialog" doesn't make that distinction clearer.

  • votePoll (For voting in polls or quizzes and retracting votes)

There is no real action in there. Alternatives:

  • voteInPoll
  • answerPoll

On top of that, should we fare differently with quizzes and polls? MTProto considers a quiz a special type of poll and we also can't check the type before making the request (thanks @code1mountain for those infos), but from a user view maybe it'd be useful when they search for "quiz" and find everything relevant in the list.

  • joinChat (join a chat by username or chat id) * (Should we unify these 2 methods to join groups?)
  • joinChatByInviteLink (join a chat by invite link)

I agree that those should be unified and parametrized.

  • createSupergroupChat (create a supergroup or channel)

From a user view, I would like to have:

  • createSupergroup
  • createPrivateGroup
  • createChannel

Those do not include the word "chat" however, so you won't find them when searching for that.

  • sendCallbackData (Interacting with inline Keyboards)

How about click or clickInlineKeyboardButton?

  • sendInlineQuery (sending an inline query to a bot)
  • sendInlineQueryResult (selecting an inline query to send)

Both say "send" but they're kind of different 🤔

Should one be requestInlineQueryResults? Hmm, also sounds weird...

  • searchMessages (for searching messages in a chat)

Means we will also get searchChats (or searchDialogs) later, right? Is this for search inside a chat or globaly?

Do we need searchGlobal that will combine like the clients do?

@giuseppeM99
Copy link
Collaborator

@JosXa dialogs are so low level they're not even mentioned in tdlib's methods, as this is an abstraction layer built on top of tdlib, i don't think we should go deeper and re-introduce another concept of dialog that will inevitably be different than the low level mtproto counterpart

@JosXa
Copy link

JosXa commented Dec 12, 2020

@giuseppeM99 Ah, I wasn't aware! That's why the other mtproto libs have it, understood.

@spontanurlaub
Copy link
Collaborator Author

@andrew-ld The joinchat method only returns ok, while the joinChatByInviteLink returns a chat object. Should I unify it by returning a chat object anyway?

@andrew-ld
Copy link
Member

@code1mountain yes

@luckydonald
Copy link
Collaborator

luckydonald commented Dec 13, 2020

  • sendCallbackData (Interacting with inline Keyboards)

How about click or clickInlineKeyboardButton?

I would go with sendCallbackData, as the Bot API also calls it callback data everywhere.
I'd just have the endpoint description say "Clicks a button" instead.

@luckydonald
Copy link
Collaborator

  • createSupergroupChat (create a supergroup or channel)

From a user view, I would like to have:

createSupergroup
createPrivateGroup
createChannel
Those do not include the word "chat" however, so you won't find them when searching for that.

how about an unified

createChat

Field Type Description
type String Type of chat, can be either “private”, “group”, “supergroup” or “channel”

this would simplify the search, and also be consistent with the Chat object.

@JosXa
Copy link

JosXa commented Dec 13, 2020

  • sendCallbackData (Interacting with inline Keyboards)

How about click or clickInlineKeyboardButton?

I would go with sendCallbackData, as the Bot API also calls it callback data everywhere.
I'd just have the endpoint description say "Clicks a button" instead.

Thinking about it, as we cannot emulate "clicking on a URL button", I agree with centering this around callback data. However what's weird to me is that "sending" is usually around messages, not machine-specific implementation details. Therefore, I would urge to find a name similar to fireCallback, executeCallback or invokeCallback, as breaking that consistency would be very unintuitive.

It is named "the data of a callback" (aka payload) after all, so "calling back" is what this method should represent.

My favorite would be executeInlineCallback I think.

@JosXa
Copy link

JosXa commented Dec 13, 2020

Same should go for inline queries I feel, where "sending" is absolutely valid for an individual inline query result, but firing the @inline query should absolutely be named with the same verb that we decide to use for callback_data.

@luckydonald
Copy link
Collaborator

With answerCallbackQuery being how the bot acts on them maybe askCallbackQuery?

@spontanurlaub
Copy link
Collaborator Author

how about an unified

createChat

Field Type Description
type String Type of chat, can be either “private”, “group”, “supergroup” or “channel”
this would simplify the search, and also be consistent with the Chat object.

Creating a basic group requires a title and a list of members to be added (Not sure if it can be an empty list, but I assume).

Creating a supergroup or channel requires a title, a description (may be empty) and optionally a location.

So a createChat Method would have these parameters:

  • type: one of group, supergroup, channel
  • title: String title
  • longitude/latitude: location parameters, only works for supergroups

Adding members to a group can be done later, it's also not the best way to get people to join your group.

@spontanurlaub
Copy link
Collaborator Author

spontanurlaub commented Dec 13, 2020

For ReportChat I would have a string parameter called reason. If it is equal to some predefined values, it will default to the Copyright, Spam, Violence... reasons. If it isn't equal to one of these default Values, it will send a chatReportReasonCustom with the provided string as text.

@JosXa
Copy link

JosXa commented Dec 13, 2020

joinchat?joinlink=xxxx

joinlink -> invite_link ?

@JosXa
Copy link

JosXa commented Dec 13, 2020

@code1mountain

Creating a basic group requires a title and a list of members to be added (Not sure if it can be an empty list, but I assume).

In the native clients that's not possible. Can someone confirm using raw mtproto?

@giuseppeM99
Copy link
Collaborator

as far as i remember, you can't create an empty group
you can however create empty supergroups (megagroups)

@luckydonald
Copy link
Collaborator

So we needs something like a initial_members array.
Would that just be the same stuff as the usual chat_ids, so either int id or @username?

Should we support the same argument for other groups, too?
While we could only accept it for groups, I bet someone would like to make a fork enabling support for that anyway if we don't.

@spontanurlaub
Copy link
Collaborator Author

I added an alias authpassword for 2fapassword, because 2fapassword is not a valid name for functions or methods in many programming languages and I want to simplify the naming of new functions. Only authpassword is mentioned in the docs, but 2fapassword still works.

@spontanurlaub spontanurlaub marked this pull request as ready for review December 16, 2020 09:44
@spontanurlaub
Copy link
Collaborator Author

I will not add any new features to this pull request and the documentation is pretty much ready (just some discussion on how/where to deploy it, that's why I have it here for now).

Please review and test the code, so we can deploy the documentation and this Pull Request at the same time?

PS: Can someone please help me merging the conflicts? Never done this before.

@luckydonald
Copy link
Collaborator

@code1mountain Please make sure it's authPath and the same camelCase for the login in the docs, so we stay consistent

# Conflicts:
#	telegram-bot-api/Client.cpp
#	telegram-bot-api/Client.h
@MarcoBuster MarcoBuster changed the title [WIP] User only methods User only methods Dec 16, 2020
@MarcoBuster
Copy link
Collaborator

(Let's not use [WIP] in titles, use the "draft PR" feature)

telegram-bot-api/Client.cpp Outdated Show resolved Hide resolved
TDLight bot api automation moved this from In progress to Review in progress Dec 16, 2020
@spontanurlaub
Copy link
Collaborator Author

I added message scheduling to this commit, because it took longer to merge than expected and I used some code from the other added methods. All send methods have now an optional parameter send_at, which can either be a unix timestamp or online (to send when the other chat participant comes online).

Scheduled messages have a negative message_id to destinct them from normal message_ids. Message has 2 new optional parameters now, is_scheduled and scheduled_at. There are also 2 new methods getScheduledMessages and editMessageScheduling.

The existing methods editMessageText, editMessageCaption, editMessageMedia and deleteMessage also work exactly the same to edit/delete scheduled messages. I will add the documentation to my other PR soon.

Copy link
Collaborator

@giuseppeM99 giuseppeM99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also some formatting issues, use clang-format to fix them automatically

telegram-bot-api/Client.cpp Outdated Show resolved Hide resolved
telegram-bot-api/Client.h Outdated Show resolved Hide resolved
telegram-bot-api/Client.cpp Outdated Show resolved Hide resolved
if (arg < 0) {
return 0;
}
// if (arg < 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove this check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scheduled messages have negative message ids. Therefore this check if the message_id is negative breaks scheduled messages.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but this could possibly break other methods that expect the message_id to be positive, especially with bots that can't schedule messages, or methods that should work with non scheduled messages, adding a parameter to allow negative numbers would be fine for me

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check basically moved to as_tdlib_message_id. There it uses the old conversion (20 bit shift) only for positive message_ids. It still has the same check for positive numbers and an other behaviour for negative ones.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arg < 0 && is_bot

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work because get_message_id is a static function.

telegram-bot-api/Client.cpp Show resolved Hide resolved
telegram-bot-api/Client.cpp Outdated Show resolved Hide resolved
@MarcoBuster MarcoBuster self-requested a review December 26, 2020 17:39
@MarcoBuster
Copy link
Collaborator

@code1mountain what's the status of this? Are you just waiting for our review?

@spontanurlaub
Copy link
Collaborator Author

Yes, mainly waiting for review.

TDLight bot api automation moved this from Review in progress to Reviewer approved Jan 2, 2021
@MarcoBuster MarcoBuster merged commit 954bc41 into tdlight-team:master Jan 2, 2021
TDLight bot api automation moved this from Reviewer approved to Done Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants