Repository navigation
fix(chats): keep pin state local to each folder - #275
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Automated PR reviewHead: Review unavailable: Incomplete model answer (length); manual review required. Remaining gates
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. |
c1db1e3 to
bebc155
Compare
|
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. |
bebc155 to
5646022
Compare
|
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. |
5646022 to
d1bbdd0
Compare
iebb
left a comment
There was a problem hiding this comment.
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.
|
Merged the exact validated result |
Change
Chat-list pin state was stored once per chat (
ChatSummary.isPinned, the Main list'schatPosition.is_pinned), while folder positions only fed_folderOrders. Every folder projection, row, and action therefore read the Main list's pin flag:The model now keeps pin state per folder list (
_folderPins, mirroring_folderOrders' lifecycle: rawpositions,updateChatPositionwith the flag, order0,updateChatRemovedFromList, and chat removal all maintain it; a position withoutis_pinnedkeeps what the folder already recorded). Folder projections sort pinned-first within the folder's own order band. Row rendering takes an optionalpinnedoverride (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.togglePincaptures the chat list at action start, so thetoggleChatIsPinnedrequest, 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.
allowTelegramTosViolations(I_DONT_FUCKING_CARE_ABOUT_TOS), disabled in official builds, and available only in an explicitly enabled self-build.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:
toggleChatIsPinnedkeeps 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 ownchatPosition.is_pinnedper 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.
dart format --output=none --set-exit-if-changed lib test integration_test test_driver.flutter analyze.flutter test --concurrency=4.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). Headbebc1553, rebased onto current mastera0611e6a. (Earlier headed1e6553carried the same diff; a first CI run hit apumpAndSettletimeout in the untouchedretained_downloads_test.dartsuite — 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.)test/chat_folder_pin_state_test.dartdrives the realChatListViewagainst 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 nochat-row-pinnedglyph 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 issuestoggleChatIsPinnedwithchat_list = chatListFolder(1)and the folder-localis_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_swipeRowback to passingpinned: null(folder rows readingchat.isPinnedagain) re-reddened the first case with the sameExpected: <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 applicablehere.Not applicable
Agentic reviewer confirmation
Agentic code reviewer only: record this confirmation in the PR review or check these boxes before approval.