Skip to content

fix(chats): keep pin state local to each folder - #275

Merged
github-actions[bot] merged 1 commit into
iebb:masterfrom
TiaraBasori:fix/folder-local-pin-state
Oct 24, 2026
Merged

github-actions[bot] merged 1 commit into
iebb:masterfrom
TiaraBasori:fix/folder-local-pin-state

Conversation

@TiaraBasori

@TiaraBasori TiaraBasori commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Change

Chat-list pin state was stored once per chat (ChatSummary.isPinned, the Main list's chatPosition.is_pinned), while folder positions only fed _folderOrders. Every folder projection, row, and action therefore read the Main list's pin flag:

  • a chat pinned in the Main list but not in a folder rendered with the pin glyph and pinned-row colour inside the folder;
  • a chat pinned inside a folder rendered unpinned there;
  • swiping the pin action inside a folder toggled the folder's server-side pin but optimistically wrote and displayed the Main list's flag (unpinning it there visually);
  • the swipe tray, desktop context menu, and long-press preview labels read the Main-list flag from folder rows.

The model now keeps pin state per folder list (_folderPins, mirroring _folderOrders' lifecycle: raw positions, updateChatPosition with the flag, order 0, updateChatRemovedFromList, and chat removal all maintain it; a position without is_pinned keeps what the folder already recorded). Folder projections sort pinned-first within the folder's own order band. Row rendering takes an optional pinned override (null keeps the Main-list behaviour verbatim), and the folder list, folder swipe peek, swipe tray, desktop menu, and long-press preview all pass the pin state of the list the user is looking at. togglePin captures the chat list at action start, so the toggleChatIsPinned request, the optimistic update, and the failure rollback all act on that same list and cannot land in a folder selected mid-flight.

Main-list behaviour is unchanged: same rendering, same sorting, same requests. Affected platforms: all (shared chat-list code).

Telegram API Terms classification

Follow the contribution and agentic review rules. Select exactly one classification and explain it below.

This is an agentic source-code review classification. Every store version must remain compliant with applicable Telegram API Terms; the guarded-source exception does not apply to store distribution or review.

  • Compliant/unchanged: this change complies with applicable Telegram API Terms, preserves Telegram's basic functionality, and does not introduce behavior covered by section 1.4.
  • Section 1.4 exception: the entire feature is guarded by allowTelegramTosViolations (I_DONT_FUCKING_CARE_ABOUT_TOS), disabled in official builds, and available only in an explicitly enabled self-build.
  • Disallowed or unresolved: this PR must not be approved or merged until its classification and implementation is resolved.

Reason and affected Telegram behavior:

This is a client-side display/state correctness fix for Telegram's own chat folders. It sends no new request types and changes no Telegram state semantics: toggleChatIsPinned keeps addressing the chat list the user is acting on, with the same payload as before. Nothing is sent on the user's behalf, no status is forged or suppressed, and the folder pin state comes from Telegram's own chatPosition.is_pinned per chat list.

Basic checks (every PR)

Check each item after verification. If a check is inapplicable, explain why in the evidence below before checking it.

  • I reviewed the diff for correctness, unintended behavior, secrets, and unrelated changes.
  • Formatting passed, or I recorded why it is inapplicable: dart format --output=none --set-exit-if-changed lib test integration_test test_driver.
  • Analysis passed, or I recorded why it is inapplicable: flutter analyze.
  • Relevant tests passed, or I recorded why they are inapplicable; name the commands and test cases below. Quality CI runs flutter test --concurrency=4.
  • I checked affected build/platform paths and applicable localization/generated files; list the evidence below.
  • I completed the Terms classification above and verified that official builds cannot enable section 1.4 exceptions.

Commands, results, platform/build/UI evidence, and reasons for inapplicable checks:

Toolchain: Flutter 3.44.2 (/tmp/flutter-3.44.2, the version Quality CI pins). Head bebc1553, rebased onto current master a0611e6a. (Earlier head ed1e6553 carried the same diff; a first CI run hit a pumpAndSettle timeout in the untouched retained_downloads_test.dart suite — green on this box and green on the sibling PRs' full runs — so the branch was re-run on a fresh head rather than changed.)

dart format --output=none --set-exit-if-changed lib/chats/chat_list_view_model.dart \
  lib/chats/chat_list_view.dart lib/chats/chat_row_view.dart \
  lib/chats/chat_list_preview.dart test/chat_folder_pin_state_test.dart
→ 0 changed, exit 0

flutter analyze lib/chats/chat_list_view_model.dart lib/chats/chat_list_view.dart \
  lib/chats/chat_row_view.dart lib/chats/chat_list_preview.dart \
  test/chat_folder_pin_state_test.dart
→ No issues found!

flutter test --no-pub -j 1 test/chat_folder_pin_state_test.dart        → 2 passed
flutter test --no-pub -j 1 test/chat_folder_transition_test.dart        → 4 passed
flutter test --no-pub -j 1 test/chat_list_in_place_update_test.dart     → 6 passed
flutter test --no-pub -j 1 test/folder_unread_badge_test.dart           → 8 passed
flutter test --no-pub -j 1 test/folder_badge_metadata_test.dart         → 1 passed
flutter test --no-pub -j 1 test/chat_list_preview_test.dart             → 34 passed
flutter test --no-pub -j 1 test/chat_row_folder_tags_test.dart          → 10 passed

python3 tool/check_l10n_strings.py                 → OK
python3 translations/tools/check.py                → OK (8 locales x 3936 keys)
python3 translations/tools/gen_assets.py --check   → OK
python3 tool/gen_registration_anchors.py --check   → OK

test/chat_folder_pin_state_test.dart drives the real ChatListView against a TdClient proxy. Two cases:

  • folder rows render folder-local pin state — two chats in folder 1, one Main-pinned/folder-unpinned, one Main-unpinned/folder-pinned. With the folder selected, the Main-pinned chat renders no chat-row-pinned glyph and a plain background; the folder-pinned chat renders the glyph and the pinned row colour. Also asserts the folder swipe peek shows the target folder's pin state.
  • toggling a folder row pins only that folder list — pinning/unpinning a row inside the folder issues toggleChatIsPinned with chat_list = chatListFolder(1) and the folder-local is_pinned, and the Main list's pin state for that chat is untouched afterwards.

Mutation checks (quoted): before the fix, both cases were red — folder rows render folder-local pin state: Expected: <0> Actual: <1> (the Main-pinned chat rendered the folder pin glyph); toggling a folder row pins only that folder list: Found 1 widget with key [<'chat-row-pinned'>] after the folder toggle back in the Main list. After the fix, both green. Mutating _swipeRow back to passing pinned: null (folder rows reading chat.isPinned again) re-reddened the first case with the same Expected: <0> Actual: <1>; restored, green again.

Build/platform paths: Dart-only change in the shared chat-list code; no plugin, asset, permission, or native code, so no build path is touched. No localization keys added (no new user-visible strings; action labels reuse existing keys).

Section 1.4 exception checks

Complete every item for an exception. For a compliant/unchanged PR, write Not applicable here.

Not applicable

Agentic reviewer confirmation

Agentic code reviewer only: record this confirmation in the PR review or check these boxes before approval.

  • I independently confirmed the Terms classification and inspected all required guards and evidence.
  • Applicable checks and Quality CI pass; no unresolved classification or unguarded section 1.4 behavior remains.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-24T00:49:54.029674Z
We couldn’t complete this review. Comment @codex review to try again.
ed1e655 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Automated PR review

Head: 5646022a31e71e0c35fced4d97ba2651a600a652 · master: ba7c3b3b7d267c3b6f61eb8dbdab8a875cb8afe1
LLM recommendation: needs_human. This is not a GitHub approval.
Auto-merge: blocked. Only iebb can authorize the current head.

Review unavailable: Incomplete model answer (length); manual review required.

Remaining gates

  • Waiting for iebb to approve this exact current head.
  • Quality must attest the exact live master/head merge. Update the PR branch/commit for a fresh pull_request Quality run, then approve that head.
  • master source incomplete: lib/chats/chat_list_preview.dart
  • head source incomplete: lib/chats/chat_list_preview.dart
  • master source incomplete: lib/chats/chat_list_view.dart
  • head source incomplete: lib/chats/chat_list_view.dart
  • master source incomplete: lib/chats/chat_list_view_model.dart
  • head source incomplete: lib/chats/chat_list_view_model.dart

No PR code runs with the model key or write token. Free-only routing; provider/rate-limit failures never authorize a merge. Existing review findings must be resolved, not hidden by a new recommendation.

@TiaraBasori
TiaraBasori force-pushed the fix/folder-local-pin-state branch 2 times, most recently from c1db1e3 to bebc155 Compare October 24, 2026 02:00

iebb commented Oct 24, 2026

Copy link
Copy Markdown
Owner

Source review of bebc155 found the folder-local pin state fix net positive; no concrete blocker found in the inspected diff. Before approval/publication, please update against current master 0c616aa and let ordinary pull_request Quality verify the fresh merged result. The last green attestation is on older master 3c9fcb1, and GitHub rejects the maintainer account updating this contributor branch. Useful follow-up coverage: delayed pin failure while switching folders, position removal, and folder deletion/recreation. No approval or merge implied by this comment.

@TiaraBasori
TiaraBasori force-pushed the fix/folder-local-pin-state branch from bebc155 to 5646022 Compare October 24, 2026 04:38

iebb commented Oct 24, 2026

Copy link
Copy Markdown
Owner

PR #256 is now merged. Please refresh this branch against current master ba7c3b3 and let ordinary pull_request Quality verify the fresh merged result before further approval/publication. I tried the normal GitHub update-branch operation with expected head 5646022; GitHub returned 403: user does not have permission to update the head repository. This is a request for the contributor to refresh, not an approval or merge of #275.

@TiaraBasori
TiaraBasori force-pushed the fix/folder-local-pin-state branch from 5646022 to d1bbdd0 Compare October 24, 2026 06:50

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed exact head d1bbdd0 under the maintainer’s explicit delegated review authority. Confirmed all five PR-specific source/test blobs are identical to previously reviewed 5646022 after the master refresh. Rechecked folder pin ingestion/update/removal, folder projection ordering, row/peek/menu/preview presentation and toggle request/rollback targeting, plus the complete added widget tests. This is a genuine net-positive fix: Main-list pins no longer leak into folder display or optimistic folder toggles. The initial pin read occurs synchronously, while request and asynchronous rollback use the captured target.chatList. Existing Main behavior is preserved. Terms classification independently confirmed compliant/unchanged; no section 1.4 exception, native/dependency/localization changes or new request type. Applicable checklist evidence is complete. Ordinary PR Quality 40071756244 passed on exact tested merge 3c317d0; verified parents are live master ba7c3b3 and this reviewed head, with no unresolved review threads or change requests. These are source review and CI results, not newly executed local/device tests. Delayed failure/folder-switch and removal/recreation regression cases remain useful follow-up coverage, not a demonstrated blocker. Guarded publication must still revalidate every deterministic gate.

@github-actions
github-actions Bot merged commit 3c317d0 into iebb:master Oct 24, 2026
3 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Merged the exact validated result 3c317d0c21dc8fb317c263f0f5f9356f0194137e into master after rechecking iebb's current-head approval. Head d1bbdd0770645bae6c9404185f012f492ad14349, prior master ba7c3b3b7d267c3b6f61eb8dbdab8a875cb8afe1. Full merged-result Quality run 40071756244 passed before publication; this job verified its attestation and did not execute PR code. Post-merge Quality has been requested separately; this comment does not claim it has passed.

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

Successfully merging this pull request may close these issues.

2 participants