Code reorganization #41
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…compose core Split code into core and telegram front-end modules, decomposed core into submodules to be used by each other and front-end. WARNING: this is not a completed action, just one step. The code currently has some terrible parts and is not even supposed to work correctly all together. There are other steps to be performed. Besides, the `core.invitations` is not decomposed well enough, it is yet to be reorganised further (to be split into back-end and front-end parts, among other things), but this is probably not a subject to this patch sequence. Although, who knows
Decomposed telegram front-end part, rearranged code, fixed some imports (both in telegram and core parts). The code seems to work now [but it now requires a VERY strange actions to make it run], though it has various shitty parts yet. There are things to decompose further (like the invitations system), and I'm planning to do some cosmetic changes (reorganize files themselves, rename something, reorder imports) after I'm done with the substantive part
Updated core module files to use OOP. This is to make interaction of front-end and back-end simpler (by using methods, which associated with a particular object, instead of using functions, which are free and aren't associated with anything) and extend potential use cases (for example, with classes it would be much easier to have multiple bots running in the same application). From now on, `core` doesn't have a config file, all the data must be provided when initializing classes (there'll be more info, see #23). At the moment the worst parts are db_connector (not going to fix it soon, see #34) and invitations, which must be split into back-end and front-end parts (not sure, how soon this is going to happen). WARNING: this code is not working, because the front-end part was not updated yet to comply with the back-end's updates (made in this commit)
Made all the controllers' methods accessible directly from a `ChatBotCore` object (pseudo-inheritance, as I call it). This is done in order to let `ChatBotCore` "override" some methods of controllers to provide additional features (see the comments)
Made use of the new core (`ChatBotCore` object, more precisely) in the telegram front-end, updated references to core functions. Moved (and renamed) `conversation_starter_finisher_lock` to `ConversationsController`, wrote some docs on it. However, the way it works now may yet be a subject to change when working on #37. WARNING: the code is not working now due to circular imports. This could be easily resolved by decomposing invitations, but I'm not sure if it's the right thing to do right now. The patch seems to be getting very big already... We'll see tomorrow
Updated `InvitationsController` to work with callback functions instead of direct `telebot` interaction. Changed `ChatBotCore`'s constructor accordingly
…invitations` Also renamed these and that things to be more explicit about callbacks (core callbacks vs telegram callbacks). Might be treated as a part of code reorganization step 5/5 (coming soon :) )
Fixed call to `object.__getattr__`, incorrect invitations sending and an acceptation deadlock
kolayne
force-pushed
the
code_reorganization
branch
from
0d1d14d to
edd01d1
Compare
Renamed some variables/files/fields, slightly changed docs. Fixed a bug (!) in `telegram_bot.common.nonfalling_handler`
Moved all the python files to a python package directory, so that it's possible to correctly run the project (as a python module)
kolayne
force-pushed
the
code_reorganization
branch
from
edd01d1 to
a66f91e
Compare
kolayne
commented
Jun 5, 2021
|
|
||
| Retrieves both telegram and local identifiers of both client and operator of the conversation, in which the | ||
| given user takes part. This function can be used for understanding whether current user is a client or an | ||
| operator in his conversation, getting information about his interlocutor, etc |
There was a problem hiding this comment.
Suggested change
| operator in his conversation, getting information about his interlocutor, etc | |
| operator in his conversation, retrieving information about his interlocutor, etc |
| def get_conversing(self, tg_id: int) -> Union[Tuple[Tuple[int, int], Tuple[int, int]], | ||
| Tuple[Tuple[None, None], Tuple[None, None]]]: | ||
| """ | ||
| Get client and operator from a conversation with the given identifier |
There was a problem hiding this comment.
Suggested change
| Get client and operator from a conversation with the given identifier | |
| Get client and operator from a conversation with a user with the given identifier |
| operator in his conversation, getting information about his interlocutor, etc | ||
|
|
||
| :param tg_id: Telegram identifier of either a client or an operator | ||
| :return: If there given user is not in conversation, `((None, None), (None, None))` is returned. Otherwise a |
There was a problem hiding this comment.
Suggested change
| :return: If there given user is not in conversation, `((None, None), (None, None))` is returned. Otherwise a | |
| :return: If the given user is not in conversation, `((None, None), (None, None))` is returned. Otherwise a |
Comment on lines
55
to
59
| try: | ||
| a, b, c, d = cursor.fetchone() | ||
| except TypeError: | ||
| return (None, None), (None, None) | ||
| return (a, b), (c, d) |
There was a problem hiding this comment.
This looks better (and more precise) to me:
Suggested change
| try: | |
| a, b, c, d = cursor.fetchone() | |
| except TypeError: | |
| return (None, None), (None, None) | |
| return (a, b), (c, d) | |
| result = cursor.fetchone() | |
| if result is None: | |
| result = (None, None, None, None) | |
| a, b, c, d = result | |
| return (a, b), (c, d) |
| @@ -11,7 +11,7 @@ | |||
| 'client_id': 'CID', 'conversation_acceptation': 'ca'} | |||
|
|
|||
|
|
|||
| def contract_callback_data(d: Dict[Any, Any], converter: Optional[Dict[Any, Any]] = None) -> Dict[Any, Any]: | |||
| def shorten_callback_data(d: Dict[Any, Any], converter: Optional[Dict[Any, Any]] = None) -> Dict[Any, Any]: | |||
There was a problem hiding this comment.
Docstring below is invalidated
| mood = d.get('mood') | ||
| if mood == 'worse': | ||
| operator_tg, operator_local = d['operator_ids'] | ||
| conversation_end = datetime_from_local_epoch_secs(d['conversation_end_moment']) |
Comment on lines
31
to
33
| notification_text = "Клиент чувствует себя хуже после беседы с оператором {}, которая завершилась в {}".format( | ||
| f"[{operator_local}](tg://user?id={operator_tg})", conversation_end | ||
| ) |
Comment on lines
+48
to
+49
| # TODO: There are a private member access (`core._operators_invitations_messages`) and a race condition | ||
| # (conversation can begin after the if statement) here. Both will be fixed with #37 |
There was a problem hiding this comment.
Note: this race condition won't lead to any incorrect behavior, the user will just see the wrong error message
| "нет собеседника") | ||
| return | ||
|
|
||
| interlocutor_id = client_tg if message.chat.id != client_tg else operator_tg |
There was a problem hiding this comment.
Why not
Suggested change
| interlocutor_id = client_tg if message.chat.id != client_tg else operator_tg | |
| interlocutor_id = operator_tg if message.chat.id == client_tg else client_tg |
Comment on lines
+120
to
+123
| query = "INSERT INTO reflected_messages(sender_chat_id, sender_message_id, receiver_chat_id, " \ | ||
| "receiver_message_id) VALUES (%s, %s, %s, %s)" | ||
| cursor.execute(query, (message.chat.id, message.message_id, sent.chat.id, sent.message_id)) | ||
| cursor.execute(query, (sent.chat.id, sent.message_id, message.chat.id, message.message_id)) |
There was a problem hiding this comment.
This definitely needs a comment
Added some comments, fixed typos and slightly changed code in some places
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A great code reorganization. Lots of code rewritten, added, and rearranged. A very nice project tree!
Unfortunately, it's not yet ready to be merged into master, because it introduces a new bug, which will be fixed with #37. Hopefully, very soon.
But I want to have a PR for this anyway, to at least have all the code written so far reviewed (both by me and Codacy)
Note: after merging the PR into
master, the following issues should be edited: #22, #33