feat: add /hive command to create tasks on a remote Hive site - #5
Merged
NagariaHussain merged 5 commits intoAug 5, 2026
Merged
Conversation
mainshivam
force-pushed
the
feat/hive-task-command
branch
2 times, most recently
from
July 29, 2026 15:58
71587d1 to
4b203f5
Compare
CI runs pre-commit with --all-files, and the committed tree did not satisfy the repo's own ruff config, so every pull request failed the linter before reaching its own changes. The linter job only runs on pull_request, which is why this drift was never caught on develop. Formatting only, plus RUF005 (use iterable unpacking instead of list concatenation) in confirm_buttons and the two _summary_buttons helpers. No behaviour changes.
The bot runs on our HR site, which has no Hive doctypes. This adds a
/hive command that creates a Hive Task on a separate Frappe site over
its REST API, so neither app has to be installed alongside the other.
Configuration lives in a new Hive Sites table on BWH Bot Settings: one
row per remote site with its URL, API key and API secret. The secret is a
Password field, so it is encrypted at rest and never rendered.
The conversation is unchanged in shape: title, optional description,
start date, end date, project, then a multi-select assignee picker.
Projects and members are fetched from the remote site rather than queried
locally, and the chosen project is labelled by title rather than id.
If more than one site is configured the flow asks which one first, so the
single-site case stays exactly as many steps as before.
Notes on the remote calls (bwh_bot/hive_client.py):
- Auth is the standard `Authorization: token key:secret` header.
- Requests time out after 10s, since they run inside a Telegram webhook
and must not hold the worker open on an unreachable host.
- Assignees go through frappe.desk.form.assign_to.add, which is how Hive
tracks them: they live in `_assign`, not a child table.
- Failures raise HiveSiteError carrying the site label and the remote
message, never a traceback. Assignment is best effort, so the task is
still reported as created if only the assign step fails.
- The site row is re-read on each step, because a conversation outlives
the request that started it and the row may be edited or disabled.
Also fixes the command descriptions in the Telegram menu. Every
conversation registers the same bound BotConversation.handle_command, so
storing the description on the function object meant each registration
overwrote the previous one and all commands showed the last one ("Apply
for Work From Home"). Descriptions now live in a dict keyed by command.
BotConversation gains `requires_employee` (default True, so leave, WFH
and petty cash are unchanged) because creating a remote task does not
need the sender resolved to an Employee.
Adds bwh_bot/tests/test_hive.py: 30 tests covering the conversation, site
selection, remote failure handling and the REST client. Both Telegram and
HTTP are stubbed, so they run on any site.
mainshivam
force-pushed
the
feat/hive-task-command
branch
from
July 29, 2026 20:37
4b203f5 to
69f3b7c
Compare
mainshivam
changed the title
mainshivam
marked this pull request as draft
July 29, 2026 20:38
install-app failed outright on a site without Frappe HR, because after_install created a custom field on Attendance Request. CI installs bwh_bot on a bare site, so the Server job never reached the tests. _make_custom_fields now skips doctypes that are absent. after_migrate re-runs it, so the field still appears if HR is installed later. Adds tests for the paths that had none: - the install guard itself, so this cannot regress silently - the "Custom date..." replies, which route through the base class into on_text_input rather than on_action: custom start, custom end, an end date before the start, and an unparseable date - the unknown-action fallback - hive_client.list_members query shape and active_sites skipping disabled rows - a non-JSON response, and error text falling back to exc_type without leaking the traceback 42 tests, all passing on a site with HR and Hive, and verified that the install hooks run clean on a site with neither.
The guest-whitelisted-method rule blocks on hook(). Telegram calls the webhook unauthenticated by design, so it cannot require a session: the shared secret header is what authenticates it, and the chat whitelist gates each update before any handler runs. Documented and marked nosemgrep, as the rule asks.
mainshivam
marked this pull request as ready for review
July 29, 2026 21:17
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+14
to
+16
| # Keyed by command rather than stashed on the function: every conversation | ||
| # registers the same bound BotConversation.handle_command, so an attribute | ||
| # on the function is shared and the last registration wins for all of them. |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+32
to
+33
| # Importing the handler modules is what registers them: ping registers a plain | ||
| # command, and the conversation subclasses self-register via __init_subclass__. |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+45
to
+48
| # Telegram calls this webhook unauthenticated by design, so it cannot require a | ||
| # session. The shared secret checked below (X-Telegram-Bot-Api-Secret-Token) is | ||
| # what authenticates the request, and each update is gated again by the chat | ||
| # whitelist before any handler runs. |
Collaborator
Author
There was a problem hiding this comment.
Removed in 29851b6. The # nosemgrep annotation on the decorator stays, since that is what the scanner reads.
gajjug004
reviewed
Jul 30, 2026
| "message_date": message_date, | ||
| "payload": json.dumps(data, indent=2), | ||
| }) | ||
| doc = frappe.get_doc( |
There was a problem hiding this comment.
We can extract the Telegram Webhook Log into a separate function as it can be used in multiple places. For example, the same logic is repeated at line 115, which violates the DRY principle.
Collaborator
Author
There was a problem hiding this comment.
Good catch. Extracted into _log_update(data, message, telegram_user, update_type, **fields) in 29851b6. Both callers now pass only what differs — command/message_text from hook(), callback_query_id/callback_data from _handle_callback_query() — and the shared chat/user/message/payload fields are built once.
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+16
to
+17
| # Status a Telegram-created task lands in. Every other Hive Task status is a | ||
| # valid transition target from this one. |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+28
to
+29
| # Tasks are created on a remote Hive site over its API, so this flow needs | ||
| # neither Frappe HR nor Hive installed alongside the bot. |
gajjug004
reviewed
Jul 30, 2026
| # neither Frappe HR nor Hive installed alongside the bot. | ||
| requires_employee = False | ||
|
|
||
| # --- Step 1: pick a site (skipped when only one is configured) ------------ |
gajjug004
reviewed
Jul 30, 2026
| message_thread_id=message_thread_id, | ||
| ) | ||
|
|
||
| # --- Free-text steps: title, description --------------------------------- |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+105
to
+108
| # --- Date steps ---------------------------------------------------------- | ||
| # Quick-pick buttons emit `from`/`to` actions (see on_action). The base class | ||
| # routes the "Custom date..." buttons through awaiting_from_date / | ||
| # awaiting_to_date and hands the parsed date to on_text_input. |
gajjug004
reviewed
Jul 30, 2026
| self.update_state(state, "select_project", {"due_date": date_str}) | ||
| self._prompt_project(state, chat_id, message_thread_id=message_thread_id) | ||
|
|
||
| # --- Button actions ------------------------------------------------------ |
gajjug004
reviewed
Jul 30, 2026
| self.update_state(state, "select_assignees") | ||
| self._prompt_assignees(state, chat_id, message_id=message_id) | ||
|
|
||
| # --- Prompts ------------------------------------------------------------- |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+273
to
+274
| # Cache the titles so later steps can label the chosen project without | ||
| # another round trip to the remote site. |
gajjug004
reviewed
Jul 30, 2026
| message_id=message_id, | ||
| ) | ||
|
|
||
| # --- Creation ------------------------------------------------------------ |
There was a problem hiding this comment.
no need for this. please remove this comment
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+395
to
+396
| # Assignment is best-effort: the task exists on the remote site either way, | ||
| # so report success rather than leaving the user unsure what happened. |
gajjug004
reviewed
Jul 30, 2026
| parse_mode="HTML", | ||
| ) | ||
|
|
||
| # --- Helpers ------------------------------------------------------------- |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+23
to
+24
| # HR-backed flows (leave, WFH, petty cash) need the sender resolved to an | ||
| # Employee. Flows that do not touch HR doctypes set this to False. |
gajjug004
reviewed
Jul 30, 2026
| return f"{_base(site.site_url)}/app/hive-task/{task_name}" | ||
|
|
||
|
|
||
| # --- internals ----------------------------------------------------------- |
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+152
to
+153
| # Frappe reports failures as `exception`, or as `exc_type` plus an `exc` | ||
| # traceback. Prefer the human-readable keys and never surface the traceback. |
Collaborator
Author
There was a problem hiding this comment.
Simplified in 29851b6 — the loop now uses a walrus binding:
for key in ("exception", "message", "exc_type", "_server_messages"):
if value := payload.get(key):
return str(value)[:200]The explanatory comment above it is gone too.
gajjug004
reviewed
Jul 30, 2026
Comment on lines
+32
to
+35
| # The doctypes we extend ship with Frappe HR, which is optional: the bot is | ||
| # installable on a site that only uses the non-HR flows, and CI installs it | ||
| # on a bare site. Skip anything absent; after_migrate re-runs this, so the | ||
| # fields appear if HR is installed later. |
- Remove the explanatory and section-divider comments called out in review across telegram.py, hive.py, conversation.py, hive_client.py and install.py. - Extract the duplicated Telegram Webhook Log creation in hook() and _handle_callback_query() into _log_update(), with the differing fields passed by the caller. - Simplify the error-key lookup in _error_text() with a walrus binding.
Collaborator
Author
|
@gajjug004 all review comments are addressed in 29851b6, pushed just now. Comments removed — every explanatory and section-divider comment you flagged, across DRY on the webhook log — the duplicated
Verified: |
gajjug004
approved these changes
Aug 5, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Why?
The bot runs on our HR site, which has no Hive doctypes. Hive lives on a separate site. Previously
/hivewrote to local Hive doctypes, which meant both apps had to be installed together - that is not how our sites are set up.What?
/hivecreates a Hive Task on a configured remote Frappe site.Configuration is a new Hive Sites table on BWH Bot Settings: one row per remote site with its URL, API key and API secret. The secret is a Password field, so it is encrypted at rest and never rendered back.
The conversation keeps the same shape: title -> optional description -> start date -> end date -> project -> assignees (multi-select, tap to toggle) -> review -> create. Projects and members are fetched from the remote site instead of queried locally, and the chosen project is shown by title rather than id.
If more than one site is configured the flow asks which one first. With a single site it is exactly as many steps as before.
How?
bwh_bot/hive_client.pyis a thin REST client:Authorization: token key:secretheader.frappe.desk.form.assign_to.add, which is how Hive tracks them - they live in_assign, not a child table (see Hive's ownmigrate_assignees_to_assignpatch).HiveSiteErrorcarrying the site label and the remote message, never a traceback.BotConversationgainsrequires_employee(defaultTrue, so leave, WFH and petty cash are untouched). Creating a remote task does not need the sender resolved to an Employee.Also fixed
Command descriptions in the Telegram menu. Every command showed "Apply for Work From Home". Each conversation registers the same bound
BotConversation.handle_command, sofn.__func__._description = descriptionwrote to one shared function object and the last registration won for all of them./pingwas unaffected because it is a plain function. Descriptions now live in a dict keyed by command.install-appon a site without Frappe HR.after_installcreated a custom field onAttendance Requestunconditionally, so installing failed outright on any site without HR - including CI, which installs onto a bare site. Absent doctypes are now skipped;after_migratere-runs it, so the field still appears if HR is installed later.Tests
bwh_bot/tests/test_hive.py, 50 tests, the first in this app. Telegram and HTTP are both stubbed, so every test runs on any site with no Hive installed - nothing skips.on_text_inputrather thanon_action- custom start, custom end, end before start, unparseable inputexc_typewithout leaking a traceback, query shape for both lists, assignment endpointVerified
Beyond the mocked tests, the REST layer was exercised between two real sites - the bot on the HR site, Hive on another with bwh_bot not installed at all. Fetched projects and members, created a task and assigned it, then confirmed on the remote side that title, description, project, both dates, status and
_assignall landed correctly. The full Telegram flow was also driven end to end through a tunnelled webhook.Install hooks were additionally verified against a site with neither HR nor the bot's own HR doctypes present.
Note on the style commit
style:applies the ruff formatter and fixes 3 RUF005 lint errors across the app. CI runs pre-commit with--all-filesand the committed tree did not satisfy the repo's own ruff config, so every pull request failed the linter before reaching its own changes. The linter job only runs onpull_request, which is why this was never caught on develop. It is formatting only, no behaviour change, and can be split into its own PR if preferred.Worth noting for contributors: the pre-commit hook was not installed locally anywhere, which is how the drift accumulated.
pre-commit installprevents a recurrence.