Saving unfinished focus timer sessions to a task

[amikhaylin]

No description provided.

[github-actions]

Code review by ChatGPT

[github-actions]

Ниже — только по делу, что можно улучшить/исправить.

• Логика показа поповера: сейчас подтверждение предлагается при любом режиме, если focusTask.task != nil. Это приведет к предложению “сохранить сессию” даже во время паузы/перерыва. Предлагайте сохранение только для активной рабочей сессии и при наличии прогресса.
  Пример:
  if timer.mode == .work, timer.elapsed > 0, focusTask.task != nil {
  showingSaveSessionConfirmation = true
  } else {
  endSession(save: false)
  }

• Не используйте toggle() для показа диалога — это чревато гонками при быстрых тапах. Явно выставляйте true/false.
  Пример: showingSaveSessionConfirmation = true при показе; showingSaveSessionConfirmation = false при закрытии.

• Дублирование кода (reset + условный skip) в трех местах. Вынесите в один метод/функцию.
  Пример:
  private func endSession(save: Bool) {
  if save, let task = focusTask.task {
  task.tomatoesCount += 1
  // при необходимости сохранить контекст
  }
  let shouldSkip = timer.mode == .pause || timer.mode == .longbreak
  timer.reset()
  if shouldSkip { timer.skip() }
  }

• Порядок reset/skip: вы сначала вызываете reset(), затем проверяете режим для skip(). Если reset меняет режим, условие может стать всегда ложным. Снимайте флаг до reset, как в примере выше.

• UX/Компонент подтверждения: для простого “Да/Нет” лучше подойдут alert или confirmationDialog — они:

  • более нативные на iPhone (popover на iPhone часто ведет себя как sheet, а .presentationCompactAdaptation(.popover) доступен не на всех версиях iOS),
  • имеют роли кнопок (primary/destructive/cancel),
  • понятнее для доступности.
    Если оставляете popover, обработайте dismiss-сценарий: сейчас при тапе вне поповера кнопка Stop уже нажата, а действие не завершено. Добавьте onDismiss и явно решите, что делать (отменяем остановку или останавливаем без сохранения).
    Пример:
    .popover(isPresented: $showingSaveSessionConfirmation, onDismiss: {
    // например, ничего не делать и оставить таймер работать
    }) { ... }

• Доступность и тесты: добавьте .accessibilityIdentifier к кнопкам “Yes/No” внутри поповера/диалога, чтобы покрыть UI-тестами и улучшить доступность.
  Пример:
  Button("No", role: .cancel) { ... }
  .accessibilityIdentifier("SaveSessionNoButton")
  Button("Yes") { ... }
  .accessibilityIdentifier("SaveSessionYesButton")

• Именование: showingSaveSessionAlert говорит “alert”, но используется popover. Переименуйте, например, в showingSaveSessionConfirmation/Popover для ясности.

• Локализация: строки “Save the current session to a task”, “Yes”, “No”, “Stop” — в LocalizedStringKey / локализационные ресурсы.

• Персистентность: инкремент tomatoesCount напрямую из View — это бизнес-логика в UI-слое. Лучше инкапсулировать в модель/вью-модель (например, FocusTimerViewModel.saveCurrentSession()), а тут вызывать метод. Так проще тестировать и контролировать побочные эффекты (сохранение контекста, валидация, пороги длительности и т.д.). Если используется Core Data — не забыть context.save(). Если SwiftData — проверьте, что модификация идёт на главном потоке и автосохранение вас устраивает.

• Валидация “успешной” сессии: скорее всего, помидор должен считаться завершенным при выполнении условия (например, достигнута длительность рабочего интервала или > X% прогресса). Сейчас любой tap “Yes” инкрементит счётчик. Имеет смысл проверять условие до инкремента и/или поручить это таймеру/вью-модели.

• Совместимость: .presentationCompactAdaptation(.popover) — проверьте минимальную iOS. Если поддерживаете более ранние версии — оградите #available или используйте alert/confirmationDialog как универсальный вариант.

Рефакторинг кнопки Stop в итоге может выглядеть компактно:
Button {
let canOfferSave = timer.mode == .work && timer.elapsed > 0 && focusTask.task != nil
if canOfferSave {
showingSaveSessionConfirmation = true
} else {
endSession(save: false)
}
} label: {
Label("Stop", systemImage: "stop.fill")
}

И, например, confirmationDialog:
.confirmationDialog("Save the current session to a task",
isPresented: $showingSaveSessionConfirmation) {
Button("Save", role: .none) { endSession(save: true) }
.accessibilityIdentifier("SaveSessionYesButton")
Button("Don’t Save", role: .destructive) { endSession(save: false) }
.accessibilityIdentifier("SaveSessionNoButton")
}

[github-actions]

Ниже только по делу — про потенциальные проблемы и улучшения, связанные с добавленным кодом.

• Опечатка в ключе CloudStorage

  • Сейчас: @cloudstorage("timerSaveUnifinished") … — в ключе опечатка (“Unifinished”).
  • Это приведет к созданию нового независимого ключа, данные не будут читаться/синхронизироваться под ожидаемым именем. Исправьте на "timerSaveUnfinished".
  • Если версия с опечаткой уже ушла в прод, нужна миграция: считать значение из старого ключа и записать в новый, затем удалить старый. Сделать это либо в инициализации SettingsView, либо централизованно при старте приложения/внутри обертки CloudStorage.

• Консистентность с остальными настройками

  • У остальных значений задана дефолтная инициализация через UserDefaults (fallback), здесь просто false. Если цель — сохранять обратную совместимость/мигрировать старые значения, добавьте аналогичный fallback. Если настройка новая и бек-компат не нужен — ок, но лучше быть последовательным.
  • Кнопка “Reset” сейчас не сбрасывает новый флаг. Добавьте timerSaveUnfinished = false в обработчик “Reset to defaults”.

• Название ключа и свойства

  • Имя свойства timerSaveUnfinished вписывается в группу “timer*”, но семантически это не тайминг, а поведение сохранения. Более выразительно: saveUnfinishedSessionsToTask или timerSavesUnfinishedSessions. И главное — чтобы ключ и имя свойства были синхронны и без опечаток.

• Локализация

  • Строка “Save unfinished sessions to a task” — хардкод. Лучше вынести в локализуемый ключ: Text("settings.saveUnfinishedSessionsToTask") (или Toggle с LocalizedStringKey). Добавьте перевод в Localizable.strings.

• Стиль переключателя

  • .toggleStyle(.switch) — на iOS это и так дефолт. Если у вас кросс-платформа (macOS), фиксировать switch может быть ок (для консистентности с остальными), но это стоит делать везде одинаково. Либо оставить .automatic, либо обернуть в #if os(...) для платформенной консистентности.

• Синхронизация/хранилище

  • Если этот флаг не требуется синхронизировать между устройствами, подумайте о @AppStorage вместо @cloudstorage — быстрее, проще и без iCloud лагов. Если требуется кросс-девайс — CloudStorage уместен.

Пример исправления и миграции (если опечатка уже попала к пользователям):

• Исправить ключ:
  @cloudstorage("timerSaveUnfinished") private var timerSaveUnfinished: Bool = UserDefaults.standard.object(forKey: "timerSaveUnfinished") as? Bool ?? false

• Одноразовая миграция (идея):

  • При старте приложения:
    • Если в хранилище есть "timerSaveUnifinished", прочитать, записать в "timerSaveUnfinished", удалить старый.
  • Если CloudStorage — ваша обертка, добавьте туда механизм алиасов/миграций ключей.

• Сброс:
  Button {
  timerWorkSession = 1500.0
  timerBreakSession = 300.0
  timerLongBreakSession = 1200.0
  timerWorkSessionsCount = 4.0
  timerSaveUnfinished = false
  } …

Это устранит явный баг с ключом, обеспечит консистентность и корректный сброс настроек.

[github-actions]

Ниже — только по делу, применимо к удалению popoverViewController() у SFSafariExtensionHandler.

• Проверьте UX/функциональность: удаление popoverViewController() полностью отключает поповер у тулбарной кнопки. Если прежняя логика/настройки/индикаторы жили в поповере, убедитесь, что:

  • toolbarItemClicked(in:) реализован и теперь даёт понятную реакцию (действие, открытие настроек расширения, изменение badge/tooltip и т.д.).
  • Пользователь не остаётся без очевидного пути к настройкам (при необходимости вызывайте SFSafariApplication.showPreferencesForExtension).

• Консистентность с Info.plist и ресурсами:

  • Если поповер удалён намеренно, удалите неиспользуемые артефакты: класс SafariExtensionViewController, соответствующий .xib/storyboard, Target Membership у этих файлов.
  • Проверьте NSExtension/Attributes для toolbar item: не должно оставаться ссылок/настроек, предполагающих наличие поповера (иначе — вводящая в заблуждение конфигурация и потенциальные ворнинги/странное поведение).

• Тесты/валидация:

  • Прогоните сценарии: клик по тулбарной кнопке, валидация состояния (validateToolbarItem), работа на нескольких страницах/окнах. Без поповера клик должен явно что-то делать.
  • Проверьте локализации для Tooltip/Label — без поповера это становится основным каналом коммуникации.

• Архитектура и чистка:

  • Если раньше держали singleton shared у SafariExtensionViewController — удаление метода убирает его вечную ретенцию. Это плюс по памяти. Закройте и связанные зависимости/обсерверы, если они были.
  • Обновите документацию/комментарии в коде (почему поповер убран), чтобы рецидивы не возвращали его «по привычке».

• Если поповер нужен «иногда»:

  • В App Extension нельзя вернуть nil из popoverViewController(), а наличие метода делает клик по тулбару поповером по умолчанию. Условно показывать поповер нельзя — либо он есть всегда, либо убираете метод и реализуете альтернативный UX. Если нужна гибкость — подумайте о вынесении UI в контейнерное приложение/настройки и команде на их открытие из toolbarItemClicked.

• SwiftUI:

  • Если удаление связано с планом миграции на SwiftUI: помните, что SwiftUI можно вложить в SFSafariExtensionViewController через NSHostingView. Полное удаление поповера не требуется ради SwiftUI. Если же UI действительно больше не нужен — текущий шаг корректен.

Если цель — превратить тулбарную кнопку из «показа поповера» в «моментальное действие», то изменение правильное, при условии что вы подчистите ресурсы, настройки и дадите пользователю понятную обратную связь через действие/индикаторы.

[github-actions]

Вот конкретные замечания по PR:

• Критическая опечатка в ключе хранилища:

  • @cloudstorage("timerSaveUnifinished") → должно быть "timerSaveUnfinished".
  • Иначе настройка будет сохраняться/читаться под другим ключом, что приведет к несогласованности и невозможности шаринга значения с остальным кодом.

• Сброс настроек не учитывает новый флаг:

  • В обработчике кнопки Reset добавьте timerSaveUnfinished = false (или желаемое значение по умолчанию), иначе часть настроек сбрасывается, а часть нет.

• Локализация:

  • Текст Toggle захардкожен на английском. Лучше использовать ключ локализации: Toggle("settings.saveUnfinishedSessionsToTask", isOn: …) с комментарием для локализаторов. Сейчас у вас смесь жестко заданных строк и хранимых значений — приведите к единому подходу.

• Избыточный toggleStyle(.switch):

  • Для iOS стандартный стиль и так — переключатель. Если нет кастомных причин, можно убрать.

• Единообразие и защита от опечаток в ключах:

  • Вынесите ключи в единое место (enum/struct SettingsKeys), чтобы избежать подобных опечаток в будущем.
  • Пример: enum SettingsKeys { static let timerSaveUnfinished = "timerSaveUnfinished" }

• Обоснованность Cloud-синхронизации:

  • Уточните, действительно ли этот флаг должен синхронизироваться через iCloud. Многие пользовательские предпочтения имеют смысл быть локальными (@AppStorage), чтобы избежать неожиданных изменений между устройствами. Если остается @cloudstorage — ок, но это должно быть осознанным решением.

• Миграция (на случай, если опечатка уже ушла в прод):

  • Если версия с "timerSaveUnifinished" уже попадала к пользователям, добавьте одноразовую миграцию: прочитать старый ключ, записать новый, удалить старый.
  • Если нет — просто поправьте ключ до мерджа.

Предлагаемый патч (сохраняя текущую архитектуру):

• Ключ и локализация:

  • @cloudstorage(SettingsKeys.timerSaveUnfinished) private var timerSaveUnfinished: Bool = false
  • Toggle("settings.saveUnfinishedSessionsToTask", isOn: $timerSaveUnfinished)

• Сброс:

  • Внутри кнопки Reset: timerSaveUnfinished = false

• Ключи:

  • struct SettingsKeys {
    static let timerSaveUnfinished = "timerSaveUnfinished"
    // остальные ключи сюда же
    }

• Миграция (если нужна), где-то в точке инициализации настроек:

  • if let old = UserDefaults.standard.object(forKey: "timerSaveUnifinished") as? Bool {
    UserDefaults.standard.set(old, forKey: SettingsKeys.timerSaveUnfinished)
    UserDefaults.standard.removeObject(forKey: "timerSaveUnifinished")
    }

Дополнительно (не блокирующее, но к улучшению в этом файле):

• Приведите типы к предметной области: количество рабочих сессий логичнее хранить как Int, а не Double. Если UI-элемент требует Double, конвертируйте на границе UI.
• Избавьтесь от value(forKey:) для дефолтов в пользу явных API (double(forKey:), bool(forKey:)) или register(defaults:) на старте приложения, чтобы не тащить логику дефолтов в каждый @…Storage.

[github-actions]

Ниже только по делу — что может пойти не так и как улучшить.

• Опечатка и рассинхрон ключа:

  • Сейчас: @cloudstorage("timerSaveUnifinished") var timerSaveUnfinished …
  • В ключе — Unifinished (с лишней i), в имени — Unfinished. Это приведет к созданию нового ключа и «потере» уже сохраненного значения/несовместимости между таргетами.
  • Исправьте ключ на "timerSaveUnfinished" и (если ключ уже «ушел» в прод) добавьте миграцию: на старте приложения один раз прочитать старый ключ из NSUbiquitousKeyValueStore.default и перенести в новый, затем удалить старый.

• Неправильный идентификатор:

  • В блоке if timerSaveUnfinished { focusedTask.tomatoesCount += 1 } — переменная focusedTask не объявлена. Вероятно, имелся в виду либо task, либо focusTask.task.
  • Исправить, например:
    if timerSaveUnfinished {
    (focusTask.task ?? task).tomatoesCount += 1
    }
    или явнее:
    if timerSaveUnfinished, let t = focusTask.task {
    t.tomatoesCount += 1
    }

• Логика инкремента «помидора» в неверных состояниях:

  • Сейчас инкремент происходит вне зависимости от того, какую сессию вы прерываете. При .longbreak вы тоже увеличите tomatoesCount, что, скорее всего, неверно.
  • Инкрементируйте только если незавершенная сессия — фокус. Например:
    if timerSaveUnfinished, timer.mode == .focus || (timer.mode == .pause && timer.previousMode == .focus) {
    …
    }
  • Если у таймера нет такой модели состояний, добавьте явный флаг текущего типа сессии (focus/break).

• Риск двойного инкремента:

  • Проверьте, не делает ли timer.skip() уже инкремент tomatoesCount по завершению/пропуску фокус-сессии. Если да — ваш дополнительный плюс даст задвоение. Решение: либо перенести подсчет внутрь таймера и возвращать результат завершения (например, skip() -> SessionOutcome), либо инкрементировать только при skip() == .skippedFocusWithoutCompletion.

• Сохранение и обновление виджетов:

  • После изменения модели стоит явно сохранить контекст и обновить виджеты (если они зависят от tomatoesCount), иначе изменения могут не успеть попасть в виджеты до закрытия:
    try? modelContext.save()
    WidgetCenter.shared.reloadAllTimelines()

• @cloudstorage: доступность и консистентность таргетов:

  • Убедитесь, что CloudStorage доступен на минимально поддерживаемых платформах приложения/виджета (iOS 17+/watchOS 10+). Иначе добавьте @available или условную компиляцию/фолбэк на @AppStorage + NSUbiquitousKeyValueStore.default.
  • Проверьте, что те же ключи используются во всех таргетах, где они читаются.

• Нейминги ключей настроек:

  • Лучше неймспейсить ключ: например, "settings.timer.saveUnfinished" — снижает шанс коллизий и упрощает миграции/поиск.

• Архитектурно: настройку лучше вынести из View:

  • Вместо хранения @cloudstorage прямо во вью, используйте единый AppSettings (Observable/@observable) с оберткой над хранилищем. Это упрощает тестирование, миграцию ключей и устраняет дублирование по проекту.

Итоговый минимальный фикс по месту (без архитектурных переработок):

• Заменить ключ и идентификатор:
  import CloudStorage

  @cloudstorage("settings.timer.saveUnfinished")
  private var timerSaveUnfinished: Bool = false

  ...

  if timer.mode == .pause || timer.mode == .longbreak {
  timer.skip()
  }
  if timerSaveUnfinished, timer.mode == .focus {
  task.tomatoesCount += 1
  }
  try? modelContext.save()
  WidgetCenter.shared.reloadAllTimelines()
  focusTask.task = nil
  presentationMode.wrappedValue.dismiss()
  list = .focus

И дополнительно — миграция старого ключа (один раз при старте приложения), если он уже попал к пользователям.

[github-actions]

Ниже — только по делу, по изменениям из дифа.

• Орфографическая ошибка в ключе CloudStorage:

  • Сейчас: @cloudstorage("timerSaveUnifinished") var timerSaveUnfinished
  • В имени ключа опечатка ("Unifinished"). Это легко приведет к расхождению с тем же ключом в других местах, миграции настроек и т.п.
  • Что сделать:

    • Вынесите ключ в константу и исправьте на единый правильный вариант, напр. "timerSaveUnfinished".

    • Если ключ уже успел попасть в прод, добавьте простую миграцию со старого ключа на новый.

    • Минимизируйте «магические строки»:
      struct PrefsKey {
      static let timerSaveUnfinished = "timerSaveUnfinished"
      static let legacyTimerSaveUnifinished = "timerSaveUnifinished"
      }

      #if canImport(CloudStorage)
      @cloudstorage(PrefsKey.timerSaveUnfinished) private var timerSaveUnfinished = false
      @cloudstorage(PrefsKey.legacyTimerSaveUnifinished) private var legacyTimerSaveUnifinished = false
      #endif

      .task {
      if legacyTimerSaveUnifinished && !timerSaveUnfinished {
      timerSaveUnfinished = true
      legacyTimerSaveUnifinished = false
      }
      }

• Дублирование логики «завершить фокус/таймер» в двух местах:

  • Один и тот же блок: условный skip(), инкремент tomatoesCount при флаге, сброс focusTask.task.

  • Вынесите в приватный метод, чтобы не разъезжалась последовательность действий и упростить изменения:
    private func endFocusAndOptionallyCountTomato() {
    if timer.mode == .pause || timer.mode == .longbreak {
    timer.skip()
    }
    if timerSaveUnfinished {
    focus.tomatoesCount += 1
    try? modelContext.save() // см. пункт ниже
    }
    focusTask.task = nil
    }

    И вызывайте его из обоих мест.

• Сохранение изменения модели:

  • Вы увеличиваете focus.tomatoesCount, но не видно явного сохранения. Если focus — SwiftData-модель, без save() изменения могут не попасть на диск.
  • Рекомендую после инкремента вызывать try? modelContext.save() (или централизовать сохранение там, где это у вас принято, но чтобы это было гарантированно).

• Условие инкремента незавершенного помидора:

  • Сейчас инкремент происходит всегда при включенном флаге, даже если пользователь остановил помидор через несколько секунд.
  • Подумайте о пороге прогресса (например, >= 50% от рабочего интервала) перед инкрементом. Это защитит статистику от «случайных» стопов.
  • Пример:
    if timerSaveUnfinished, timer.progress >= 0.5 { ... }

• Возможное двойное начисление:

  • Проверьте, не начисляется ли tomatoesCount еще где-то при завершении/выполнении задачи (task.complete или логика таймера). Если да — вы рискуете удвоить счетчик при включенном флаге. Централизуйте доменную логику начисления «помидора» в одном месте (например, в сервисе/модели таймера).

• Порядок действий и состояния таймера:

  • Вы делаете skip() только в режимах .pause и .longbreak. Проверьте сценарий остановки во время активной работы: нужно ли явно останавливать/сбрасывать таймер в .work? Если это ожидаемое поведение — ок; если нет — добавьте обработку .work.

• Доступность @cloudstorage:

  • @cloudstorage доступен не на всех версиях ОС. Убедитесь, что:
    • Deployment target соответствует требованиям (iOS/macOS версии, где @cloudstorage есть).
    • Включены нужные iCloud entitlement’ы (Key-Value storage).
    • При необходимости добавьте fallback (например, @AppStorage) под #if !canImport(CloudStorage), или поднимите минимальную версию платформы и явно задокументируйте это.

• Нейминг:

  • timerSaveUnfinished звучит как настройка UI. Если по сути это «засчитывать незавершенный помидор», более говорящие варианты: countUnfinishedPomodoro, creditUnfinishedPomodoro, saveUnfinishedPomodoroAsCompleted. Так меньше двусмысленности.

• Архитектура:

  • Логику изменения доменных моделей (инкремент, завершение, сохранение) лучше держать вне ViewModifier, в отдельном слое (ViewModel/сервис таймера). Так меньше шанс рассинхрона при расширении UI и проще тестировать.

Итого: критично — исправить ключ CloudStorage (с миграцией), вынести дублированный код в функцию, гарантировать сохранение изменений модели и пересмотреть условие начисления «незавершенного помидора», чтобы не портить статистику. Дополнительно — проверить доступность @cloudstorage и порядок остановки таймера в разных режимах.

[github-actions]

Ниже — только существенные замечания по диффу.

• Опечатка в ключе CloudStorage:

  • Объявлено: @cloudstorage("timerSaveUnifinished") — с ошибкой в слове.
  • Используется переменная timerSaveUnfinished — без ошибки.
  • Итог: создаётся новый ключ в облачном хранилище, настройка не совпадёт с существующей и будет всегда false у текущих пользователей. Исправьте ключ на "timerSaveUnfinished" и продумайте миграцию старого значения (прочитать по старому ключу, записать в новый, удалить старый).

• Условие инкремента помидоров некорректно:

  • Сейчас focus.tomatoesCount += 1 срабатывает при любом режиме таймера, лишь бы включена настройка. Это приведёт к инкременту во время паузы/перерыва и даже если рабочая сессия не началась.
  • Предложение:
    • Инкрементировать только если идёт рабочая сессия (введите явный признак типа timer.isWorkSession или проверяйте конкретное значение режима) и есть фактический прогресс (например, elapsed > 0).
    • Не инкрементировать в .pause/.longbreak.
    • Пример логики: if timerSaveUnfinished && timer.isWorkSession && timer.elapsed > 0 { focus.tomatoesCount += 1 }

• Возможный дабл-каунт с timer.skip():

  • Если skip() сам учитывает завершение/инкремент текущей сессии, ручной tomatoesCount += 1 даст двойной учёт.
  • Сделайте поведение взаимоисключающим: либо skip() считает, либо вы. Идеально — инкапсулировать в одно место, например timer.finishCurrentSession(countAsTomato: Bool).

• Непроверенная связь фокус‑задачи с текущей:

  • Код обнуляет focusTask.task и меняет счётчик помидоров, не проверяя, что завершается именно та же задача, которая сейчас в фокусе.
  • Добавьте проверку идентичности: if focus.focusTask?.task?.id == task.id { … }. Иначе можно случайно сбросить чужую фокус‑задачу и посчитать «незавершённый» помидор при завершении другой задачи.

• Порядок и атомарность изменений:

  • В одном обработчике вы:
    • управляете таймером,
    • меняете состояние фокуса,
    • завершаете модель Todo.
  • Рекомендации:
    • Явно определить порядок: завершить/остановить/скипнуть таймер → при необходимости посчитать незавершённый помидор → отвязать фокус‑задачу → complete(modelContext:).
    • Свести это в один «интент»/метод домена (например, completeTaskAndHandleTimer(task:)), чтобы избежать рассинхронизации и упростить тестирование.

• Жёстко прописанные строковые ключи:

  • Уже привели к багу. Вынесите ключи в именованные константы (static let) или единый enum/Namespace, чтобы исключить опечатки и упростить миграции.

• Декомпозиция условия:

  • Сейчас вложенность в Button(action:) растёт. Для читаемости используйте ранние выходы и небольшие приватные методы, например:
    • guard isCurrentTaskFocused else { task.complete(...); return }
    • handleTimerBeforeCompletingTask()
    • detachFocusTask()

• Поток исполнения:

  • Все эти изменения должны выполняться на главном потоке. Если не гарантировано, пометьте вью @mainactor или перенесите логику в @mainactor‑сервис.

Дополнительно (по желанию):

• Добавьте тесты на сценарии: завершение фокус‑задачи в режимах work/pause/shortbreak/longbreak, прерванная но не начатая сессия (elapsed == 0), и завершение не‑фокусной задачи при активном фокусе — счётчик не должен меняться и фокус не должен сбрасываться.

[github-actions]

Вот только то, что действительно стоит улучшить/проверить.

• Опечатка в ключе CloudStorage

  • Объявлено: @cloudstorage("timerSaveUnifinished") … (лишняя i).
  • Использование переменной: timerSaveUnfinished.
  • Итог: создадите новый ключ, потеряете существующее значение/синхронизацию. Вынесите ключ в константу и исправьте орфографию, чтобы исключить подобные баги.
  • Пример:
    • enum StorageKeys { static let saveUnfinishedPomodoro = "timerSaveUnfinished" }
    • @cloudstorage(StorageKeys.saveUnfinishedPomodoro) private var saveUnfinishedPomodoro = false

• Дублирование логики “skip + счетчик + сброс фокуса”

  • Один и тот же блок повторяется в двух местах. Вынесите в приватный метод, чтобы избежать рассинхронизации при будущих правках.
  • Важно: снимайте «снимок» состояния таймера до изменения (до skip), чтобы правильно решить, считать ли незаконченный помидор.
  • Пример:
    • private func finalizeFocusIfNeeded() {
      let wasFocusLike = timer.mode == .focus || timer.mode == .pause
      let hadProgress = timer.elapsed > 0
      if timer.mode == .pause || timer.mode == .longbreak {
      timer.skip()
      }
      if timerSaveUnfinished && wasFocusLike && hadProgress {
      focus.tomatoesCount += 1
      }
      focusTask.task = nil
      }

• Уточните условие инкремента незавершенного помидора

  • Сейчас tomatoesCount увеличивается даже если таймер в longbreak или pause без прогресса. Часто желаемая логика — считать только незавершенную фокус‑сессию с ненулевым прогрессом.
  • Рекомендуется проверять:
    • режим (focus или pause как «пауза фокуса»);
    • наличие прогресса (elapsed > 0);
    • возможно, минимальный порог (например, elapsed >= 60 сек), чтобы отсечь случайные старты.

• Порядок операций

  • Если skip() меняет mode/elapsed, сперва вычислите условия инкремента на основании текущего состояния (см. выше), затем вызывайте skip(), а уже потом модифицируйте модель.

• Доступность и импорты CloudStorage

  • Убедитесь, что деплоймент‑таргет поддерживает @cloudstorage (iOS 17+/macOS 14+ и включенный iCloud Key-Value Store).
  • Добавьте #if canImport(CloudStorage) и/или @available, если таргетов несколько.
  • Проверьте entitlements: iCloud + Key-Value storage.

• Сохранение изменений модели

  • Если focus — объект SwiftData/модель, убедитесь, что изменения tomatoesCount сохраняются (либо через autosave, либо явным try? modelContext.save()). Сейчас save вызывается только при task.complete(...), а инкремент — отдельно.

• Поток выполнения/MainActor

  • Из UI-экшенов вы изменяете модель SwiftData и состояние таймера. Чтобы исключить предупреждения/гонки, пометьте модификатор или методы как @mainactor, если соответствующие типы этого требуют.

• Нейминг

  • timerSaveUnfinished — ок, но лучше отражать семантику: countUnfinishedPomodoros или includeUnfinishedFocusInTomatoes, чтобы сразу было ясно, что делаем.

• Тесты/миграция

  • Если ранее ключ уже использовался (с правильным написанием), сделайте одноразовую миграцию значения со старого ключа и добавьте простые UI/юнит‑тесты на сценарии:
    • завершение задачи в разных режимах (focus/pause/longbreak);
    • наличие/отсутствие прогресса;
    • включенный/выключенный флаг CloudStorage.

Итоговые быстрой фиксы:

• Исправить ключ и вынести его в константу.
• Вынести повторяющийся код в finalizeFocusIfNeeded(), считать незавершенный помидор только при wasFocusLike && hadProgress.
• Убедиться в сохранении изменений модели и доступности @cloudstorage на целевых платформах.

[github-actions]

Ниже — только те вещи, которые, на мой взгляд, действительно требуют внимания или могут быть улучшены.

• Опечатка в ключе CloudStorage

  • @cloudstorage("timerSaveUnifinished") — опечатка в ключе (“Unifinished”). Переменная названа правильно (timerSaveUnfinished), но хранится под другим ключом. Это приведёт к тому, что настройка не будет читаться/синхронизироваться как ожидается и создаст новый ключ. Исправьте и при необходимости добавьте миграцию (прочитать старый ключ, перенести в новый, удалить старый).

• CloudStorage: зависимость и доступность

  • Убедитесь, что эта зависимость действительно нужна и поддерживается целевыми платформами. Если проект должен собираться на более старых ОС/без iCloud, предусмотрите @available или fallback (например, NSUbiquitousKeyValueStore или @AppStorage с App Group). Не тяните CloudStorage в тип, если используете флаг всего в одном месте — можно прокинуть его через Environment/Dependency injection.

• Логика остановки фокуса: дублирование и порядок операций

  • Один и тот же блок действий повторяется в двух местах: reset/skip, инкремент tomatoesCount, сброс focusTask.task. Вынесите в единый метод, чтобы не было расхождений поведения и было легче править:
    • Пример: stopFocus(saveUnfinished: Bool, onlyIfTaskIs current: Todo?) или метод на FocusTimer/менеджере фокуса.
  • Порядок reset/skip: в одном месте вы делаете skip без reset, в другом — reset перед проверкой, из‑за чего условие может уже не выполниться. Если вам нужно «прервать длинный перерыв», сначала определите состояние, затем skip, затем reset (или наоборот — но одинаково везде). Сейчас порядок различается и поведение будет отличаться.

• Несогласованность state/mode

  • В одном месте проверяете timer.mode == .pause || .longbreak, в другом — timer.state == .paused. Обычно «pause/paused» — это состояние, а не режим. Определите чёткую модель состояний/режимов и используйте её последовательно. Похоже, что проверка на .pause в mode — ошибка и должна быть state == .paused.

• Остановка фокуса при завершении «чужой» задачи

  • В обработчике завершения задачи вы сбрасываете текущий фокус без проверки, что «фокусируемая» задача совпадает с task. Это может неожиданно «грохнуть» фокус на другой задаче, если пользователь завершает «левую» карточку. Возможно, нужно:
    • Либо проверять focus == task перед сбросом,
    • Либо принимать явное продуктовое решение: при завершении любой задачи фокус останавливается — но тогда зафиксируйте это явно и покройте тестом.

• Инкремент tomatoesCount и сохранение контекста

  • Вы меняете focus.tomatoesCount, но явного save для этого изменения нет. На SwiftData автосейв не всегда предсказуем в моменте. Рекомендую либо:
    • Сохранить modelContext после пакета изменений (в том числе после task.complete, если он не сохраняет сам),
    • Либо гарантировать, что complete() сохраняет и включает этот инкремент в одну транзакцию (желательно атомарно).

• Потенциальное двойное начисление

  • Если task.complete(modelContext:) внутри тоже трогает tomatoesCount, при focus == task получите двойной инкремент. Стоит централизовать логику начисления «незавершенного помидора» в одном месте.

• «Start focus» поведение при разных состояниях таймера

  • Сейчас:
    • idle: reset + start — ок,
    • paused: resume — ок,
    • running/break: ничего не делаете, но меняете focusTask.task = task.
  • Решите, что делать, если идёт перерыв или уже идёт сессия на другой задаче: останавливать/сбрасывать/пропускать перерыв? Сейчас сценарий «переключиться на другую задачу» может оставить таймер в неподходящем режиме.

• Единообразие UI-лейблов

  • В одном месте Label(..., systemImage:), в другом — Image + Text. Мелочь, но лучше придерживаться одного стиля. Плюс, строки "Stop focus"/"Start focus" — проверьте, что вы используете локализуемые ключи, если проект локализуется.

• Анимация и UX

  • Сброс focusTask.task = nil можно обернуть в withAnimation для более плавной смены UI.

• Потенциальные требования к MainActor

  • Убедитесь, что методы FocusTimer вызываются с главного потока (желательно пометить FocusTimer @mainactor). SwiftUI action-замыкания обычно на main, но лучше соблюсти контракт в типе.

Идея рефакторинга (эскиз, адаптируйте к вашему API):

• Вынести «остановку фокуса» в один метод, и выровнять порядок действий и проверки state/mode.

Пример:

private func stopFocus(for task: Todo?, saveUnfinished: Bool) {
guard let current = focusTask.task else { return }
// Если нужно останавливать только когда останавливаем ту же задачу:
if let task, current != task { return }

let isOnBreak = timer.mode == .longbreak // или timer.isOnBreak
let isPaused = timer.state == .paused

if isOnBreak {
    timer.skip()
}
timer.reset()

if saveUnfinished {
    current.tomatoesCount += 1
}
focusTask.task = nil

do {
    try modelContext.save()
} catch {
    // Обработайте ошибку сохранения
}

}

После этого в местах вызова просто дергайте stopFocus(for: task, saveUnfinished: timerSaveUnfinished), без дублирования условий и с согласованным порядком.

И обязательно исправьте ключ на "timerSaveUnfinished" (или переименуйте переменную под реальный ключ), плюс продумайте миграцию значения, если опечатка уже попала в прод.

[github-actions]

Ниже — только конструктивные замечания по диффу строк локализаций.

• Покрытие локалей:

  • Новые ключи добавлены только для en/ru. Если проект уже поддерживает другие языки (в файле выше виден ar и др.), это приведет к частичной деградации UX: пользователи на остальных локалях увидят английский. Либо добавьте переводы для существующего набора локалей, либо явно зафиксируйте fallback-стратегию.

• Ключи "Yes"/"No":

  • По гайдлайнам Apple лучше избегать «Yes/No» в кнопках подтверждений. Предпочтительны конкретные действия: «Delete», «Discard», «Save», «Cancel». Это повышает ясность и снимает двусмысленность перевода.
  • Если всё-таки нужны «Yes/No», добавьте комментарий к ключам в .xcstrings с контекстом использования (кнопка диалога? переключатель?) — это критично для корректных переводов на другие языки.
  • Проверьте, нет ли уже в каталоге существующих ключей с «Yes»/«No», чтобы не плодить дубликаты.

• Консистентность терминов «session»:

  • В русских строках используется «сессия». В UI часто уместнее «сеанс» (особенно для тайм-трекинга/работы). Важно выбрать единый термин по всему приложению и придерживаться его. Если решите «сеанс», то:
    • "Save the current session to a task" → «Сохранить текущий сеанс в задачу»
    • "Save unfinished sessions to a task" → «Сохранять незавершенные сеансы в задачу»

• Вид/аспект глагола в русской локализации:

  • "Save unfinished sessions to a task" переведено как «Сохранять…» (имперфектив), что выглядит как название настройки/тумблера (автоматическое поведение). В английском — форма приказа «Save…», что чаще читается как действие/кнопка.
  • Определитесь с назначением:
    • Если это настройка: лучше изменить EN на «Automatically save unfinished sessions to a task» и RU на «Автоматически сохранять незавершенные сеансы в задачу».
    • Если это разовое действие: EN ок, RU стоит использовать совершенный вид «Сохранить незавершенные сеансы в задачу».

• Семантическая ясность «to a task»:

  • Фраза «…to a task» двусмысленна: в какую именно задачу сохраняется? Пользователь выбирает ее далее? Это «текущая задача»? Добавьте переводческий комментарий к ключам (например: «Appears on a button that saves the running timer entry into the currently selected task») и при необходимости уточните английскую строку («…to the selected task» / «…to the current task»).

• Единообразие стиля:

  • В EN строки — в sentence case. Убедитесь, что это соответствует общей политике проекта и не конфликтует с другими ключами (особенно в тех же категориях/экранах).

• Технически по .xcstrings:

  • Рекомендуется добавлять "comment" для каждого ключа, чтобы облегчить жизнь переводчикам и избежать контекстных ошибок.
  • Проверьте, не нужны ли вариантные формы/плюрализация для будущих кейсов (например, если появится количество сеансов). Сейчас конкретный счет не упоминается — нормально, но имейте в виду правила плюрализации для других локалей.

Краткие предложенные правки строк (если это настройка):

• EN: "Automatically save unfinished sessions to the current task"
• RU: «Автоматически сохранять незавершенные сеансы в текущую задачу»

Если это действие (кнопка):

• EN: "Save the current session to the selected task"
• RU: «Сохранить текущий сеанс в выбранную задачу»

И по возможности заменить «Yes/No» на конкретные действия в местах использования.