refactor: tag id constants + guards against unknown tag ids - #202
Open
GlebYavorski wants to merge 4 commits into
Open
refactor: tag id constants + guards against unknown tag ids#202GlebYavorski wants to merge 4 commits into
GlebYavorski wants to merge 4 commits into
Conversation
When bulk editing transactions with different categories, the modal puts a `mixed` placeholder in the tag list, and `modifyTags` is supposed to expand it into each transaction's own tags. For a transaction without any category `prevTags` is `null`, so the `id === 'mixed' && prevTags` check fell through to the `else` branch and stored the literal `mixed` string as a tag id. After that `usePopulatedTags()['mixed']` is `undefined` and rendering the transaction list crashes in `Symbol` (`tag.symbol` of undefined), taking the whole app down. Fixes ardov#147 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `mixed` placeholder was a magic string duplicated across three layers (BulkEditModal, TagChip, transaction thunks) with no hint that it is not a real tag id. Same for the `null` tag id inside those places. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lookups by tag id assumed the tag always exists, so a single unknown id (e.g. the `mixed` placeholder written by the bug in ardov#147) crashed the whole transaction list and the year review. Fall back to the null tag instead. Transactions can also hold an empty tag array once the last category is removed in the transaction editor, which `tr.tag ? tr.tag[0] : …` treated as "has a category": `getMainTag` returned `undefined` instead of `null` and CSV export crashed on `t.tag[0].title`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mock had no default export, so importing `6-shared/localization` threw and the whole suite failed to load on master. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@GlebYavorski is attempting to deploy a commit to the Alexey Ardov's projects Team on Vercel. A member of the Team first needs to authorize it. |
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
1 participant
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.
Продолжение #201 — то, что нашлось вокруг бага из #147. Ветка построена поверх #201, поэтому его коммит входит в диff; мерджить после него (или вместо — тогда
masterполучит и фикс, и остальное).1. Константы вместо магических строк
'mixed'(плейсхолдер «Разные категории» в массовом редактировании) был продублирован в трёх слоях —BulkEditModal,TagChip,transaction/thunks— и ничем не отличался от настоящего id категории, что и стало причиной #147. ТеперьmixedTagIdиnullTagIdживут в5-entities/tag/model/constants.tsи экспортируются из публичного API сущности.Литералы
'null'остались в остальном коде (envelope,budget,filteringи т.д.) — не стал раздувать диff, заменил только там, где трогал строки.2. Устойчивость к неизвестному id категории
Поиск тега по id предполагал, что тег точно существует, поэтому один незнакомый id рушил весь экран:
Transaction.Components.tsx—tags[mainTagId].symbolвалил список операций в error boundary (ровно то падение из Ошибка при добавлении категории к нескольким операциям, если в их числе есть операции без категории #147);OutcomeCard.tsx—tags[tag?.[0] || 'null'].titleтак же валил годовой обзор.Обе точки теперь падают обратно на null-тег. Это важно для тех, у кого
'mixed'уже записался в данные до фикса #201: без фолбэка они увидят краш и после обновления.3. Пустой массив категорий
tagможет быть[]— достаточно удалить последнюю категорию в редакторе операции. Проверкаtr.tag ? tr.tag[0] : …считает это «категория есть»:getMainTagвозвращалundefinedвместоnull;t.tag[0].title;getFactsгруппировал такие операции по ключуundefinedвместо null-тега.4. Починен сломанный тест
TagSelect2.test.tsxнаmasterне загружается вообще: мокi18nextбезdefault-экспорта ломает импорт6-shared/localization. Мок дополнен инстансом сuse/init/on.Проверки
npx vitest run— 30 тестов, 4 файла, всё зелёное (наmaster— 15 тестов и один упавший файл)npx tsc --noEmit— чистоnpx vite build— успешноНовые тесты:
bulkEditTransactions(обычный кейс + кейс из #147) иgetMainTag(null/[]/ несколько тегов).🤖 Generated with Claude Code