-
Notifications
You must be signed in to change notification settings - Fork 14
feat(app): add changelog dialog feature with version tracking #154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @ahmet-cetinkaya, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive changelog feature to the application, designed to inform users about new updates and improvements. It provides a dedicated dialog to display "What's New" content, which is fetched dynamically and localized. The system intelligently tracks previously shown versions, ensuring the dialog appears only for new updates, thereby enhancing user engagement and awareness of application evolution. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Reviewer's GuideImplements a localized, markdown-based changelog dialog that is shown during app initialization using new dialog and network services, tracks the last shown version in settings keyed by build number, and wires version/build metadata into the existing version bump tooling and app info. Sequence diagram for app initialization with changelog dialogsequenceDiagram
actor User
participant App
participant AppInitializationService
participant IChangelogDialogService
participant ResponsiveDialogHelper
participant ChangelogDialog
User->>App: Launch app
App->>AppInitializationService: initializeApp(navigatorKey)
AppInitializationService->>AppInitializationService: _checkAndShowOnboarding
AppInitializationService->>AppInitializationService: _checkAndShowChangelogDialog(navigatorKey)
AppInitializationService->>IChangelogDialogService: checkAndShowChangelogDialog(context)
IChangelogDialogService-->>IChangelogDialogService: Determine locale and lastShownVersion
IChangelogDialogService-->>IChangelogDialogService: Fetch changelog for locale
IChangelogDialogService->>ResponsiveDialogHelper: showResponsiveDialog(child: ChangelogDialog)
ResponsiveDialogHelper->>ChangelogDialog: Build dialog UI
ChangelogDialog-->>User: Display localized markdown changelog
User-->>ChangelogDialog: Press close button
ChangelogDialog-->>App: Navigator.pop()
AppInitializationService->>AppInitializationService: _checkAndShowSupportDialog
AppInitializationService-->>App: Initialization complete
Sequence diagram for fetching localized changelog from GitHubsequenceDiagram
participant ChangelogDialogService
participant IChangelogService
participant ChangelogService
participant AppInfo
participant GitHubRaw
ChangelogDialogService->>IChangelogService: fetchChangelog(localeCode)
IChangelogService->>ChangelogService: fetchChangelog(localeCode)
ChangelogService-->>AppInfo: Read buildNumber and version
ChangelogService-->>ChangelogService: Map localeCode to fastlaneLocale
ChangelogService->>GitHubRaw: GET /fastlane/metadata/android/fastlaneLocale/changelogs/buildNumber.txt
alt Locale specific changelog exists
GitHubRaw-->>ChangelogService: 200 OK body
else Not found for locale
GitHubRaw-->>ChangelogService: Non 200 status
ChangelogService-->>ChangelogService: Fallback to en-US
ChangelogService->>GitHubRaw: GET /fastlane/metadata/android/en-US/changelogs/buildNumber.txt
GitHubRaw-->>ChangelogService: 200 OK or error
end
alt Content retrieved
ChangelogService-->>ChangelogDialogService: ChangelogEntry(version, content)
else No content
ChangelogService-->>ChangelogDialogService: null
end
Updated class diagram for changelog dialog and servicesclassDiagram
class AppInfo {
<<static>> String name
<<static>> String shortName
<<static>> String version
<<static>> String buildNumber
<<static>> String websiteUrl
<<static>> String sourceCodeUrl
<<static>> String logoPath
}
class SettingKeys {
<<static>> String lastShownChangelogVersion
}
class ChangelogEntry {
String version
String content
ChangelogEntry(String version, String content)
}
class IChangelogService {
<<abstract>>
Future~ChangelogEntry?~ fetchChangelog(String localeCode)
}
class ChangelogService {
+Future~ChangelogEntry?~ fetchChangelog(String localeCode)
-Future~String?~ _fetchChangelogFromGitHub(String locale, String buildNumber)
-Map~String, String~ _localeMapping
-String _baseUrl
-String _fallbackLocale
}
class IChangelogDialogService {
<<abstract>>
Future~void~ checkAndShowChangelogDialog(BuildContext context)
}
class ChangelogDialogService {
-Mediator _mediator
-IChangelogService _changelogService
+ChangelogDialogService(Mediator mediator, IChangelogService changelogService)
+Future~void~ checkAndShowChangelogDialog(BuildContext context)
-Future~String?~ _getLastShownVersion()
-Future~void~ _saveLastShownVersion(String version)
}
class ChangelogDialog {
+ChangelogEntry changelogEntry
-ITranslationService _translationService
+ChangelogDialog(ChangelogEntry changelogEntry)
+Widget build(BuildContext context)
-void _launchUrl(String url)
}
class AppInitializationService {
-Mediator _mediator
-ISupportDialogService _supportDialogService
-IChangelogDialogService _changelogDialogService
-ISetupService _setupService
+AppInitializationService(Mediator mediator, ISupportDialogService supportDialogService, IChangelogDialogService changelogDialogService, ISetupService setupService)
+Future~void~ initializeApp(GlobalKey~NavigatorState~ navigatorKey)
-Future~void~ _checkAndShowChangelogDialog(GlobalKey~NavigatorState~ navigatorKey)
}
IChangelogService <|.. ChangelogService
IChangelogDialogService <|.. ChangelogDialogService
ChangelogDialogService --> IChangelogService
ChangelogDialogService --> Mediator
ChangelogDialogService --> SettingKeys
ChangelogDialogService --> AppInfo
ChangelogDialog --> ChangelogEntry
ChangelogDialog --> ITranslationService
ChangelogDialog --> AppInfo
AppInitializationService --> IChangelogDialogService
AppInitializationService --> ISupportDialogService
AppInitializationService --> ISetupService
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- In
ChangelogDialogService.checkAndShowChangelogDialogthe version-tracking logic is entirely commented out; either re-enable it (so the dialog is not shown repeatedly) or remove the dead code to make the intended behavior clear. - The locale mapping in
ChangelogServicedoesn’t include some of the locales you added translations for (e.g.,pt,no), so those users will always fall back toen-US; consider adding the missing mappings or documenting why they should fall back. - You added
AboutTranslationKeys.changelogFetchErrorbut on fetch failures the UI just logs and returns null; consider surfacing this error string in the dialog (or via a snackbar) so users get feedback when the changelog cannot be loaded.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ChangelogDialogService.checkAndShowChangelogDialog` the version-tracking logic is entirely commented out; either re-enable it (so the dialog is not shown repeatedly) or remove the dead code to make the intended behavior clear.
- The locale mapping in `ChangelogService` doesn’t include some of the locales you added translations for (e.g., `pt`, `no`), so those users will always fall back to `en-US`; consider adding the missing mappings or documenting why they should fall back.
- You added `AboutTranslationKeys.changelogFetchError` but on fetch failures the UI just logs and returns null; consider surfacing this error string in the dialog (or via a snackbar) so users get feedback when the changelog cannot be loaded.
## Individual Comments
### Comment 1
<location> `src/lib/presentation/ui/features/about/services/changelog_dialog_service.dart:28-29` </location>
<code_context>
+ // Get locale before any async operations to avoid use_build_context_synchronously warning
+ final localeCode = context.locale.languageCode;
+
+ final lastShownVersion = await _getLastShownVersion();
+ final currentVersion = AppInfo.version;
+
+ // // Don't show if this version was already shown
</code_context>
<issue_to_address>
**issue (bug_risk):** Persisted `lastShownVersion` is currently unused, so the changelog will be shown on every startup for the same version.
`lastShownVersion` is read but never used, so the dialog will appear on every launch whenever a changelog exists for the current build. If the intent is to show it only once per version (as the commented block implies), please reintroduce a guard so that when `lastShownVersion == currentVersion` the dialog is skipped.
</issue_to_address>
### Comment 2
<location> `src/lib/presentation/ui/features/about/services/changelog_service.dart:29-32` </location>
<code_context>
+ 'ro': 'ro-RO',
+ 'ru': 'ru-RU',
+ 'sl': 'sl-SI',
+ 'sv': 'sv-SE',
+ 'tr': 'tr-TR',
+ 'uk': 'uk-UA',
+ 'zh': 'zh-CN',
+ };
+
</code_context>
<issue_to_address>
**suggestion:** Locale-to-fastlane mapping may be incomplete for all supported UI locales (e.g. `no`).
The `_localeMapping` omits at least `no`, which exists as `no.yaml` in the UI. Consider aligning this mapping with all supported UI locales (including Norwegian) so users get localized changelogs instead of falling back to English where translations exist.
Suggested implementation:
```
'nl': 'nl-NL',
'no': 'nb-NO',
'pl': 'pl-PL',
```
To fully align `_localeMapping` with all supported UI locales, you should:
1. Cross-check the list of locale YAML files (e.g. in your `l10n` or `i18n` folder) or `supportedLocales` configuration.
2. Add any missing entries to `_localeMapping` so each UI locale has a corresponding fastlane directory key (e.g. `'pt'` vs `'pt-BR'`, `'zh_TW'` vs `'zh-TW'`, etc.), following the same pattern as the existing mappings.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/lib/presentation/ui/features/about/services/changelog_dialog_service.dart
Show resolved
Hide resolved
src/lib/presentation/ui/features/about/services/changelog_service.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a well-structured changelog feature, complete with a dialog, version tracking, and localization. The implementation is solid, including services for fetching changelog data from GitHub and managing its display. My review highlights a critical issue where the version-tracking logic is commented out, which would cause the dialog to appear on every app launch. I've also pointed out several opportunities to improve error handling by adding logging to silent catch blocks and suggested a refactoring to use dependency injection instead of a service locator in the new dialog widget for better testability. Addressing these points will make the feature more robust and maintainable.
src/lib/presentation/ui/features/about/services/changelog_dialog_service.dart
Outdated
Show resolved
Hide resolved
src/lib/presentation/ui/features/about/components/changelog_dialog.dart
Outdated
Show resolved
Hide resolved
src/lib/presentation/ui/features/about/services/changelog_dialog_service.dart
Outdated
Show resolved
Hide resolved
src/lib/presentation/ui/features/about/services/changelog_dialog_service.dart
Outdated
Show resolved
Hide resolved
src/lib/presentation/ui/features/about/services/changelog_service.dart
Outdated
Show resolved
Hide resolved
- implement changelog dialog component with markdown support - add changelog service to fetch localized changelog from github - create changelog dialog service with version tracking logic - add translation keys and locale support for changelog dialog - integrate changelog display in app initialization flow - update version bump script to handle build numbers - add build number constant to app info
295c284 to
3379c09
Compare
- Change network exception logging from debug to warning for better visibility - Add error logging for failed settings operations in changelog dialog service - Refactor ChangelogDialog to use constructor injection instead of service locator - Add Norwegian locale mapping for changelog localization - Remove unused import from ChangelogDialog component
Updated locale mapping to use simplified directory names following Google Play Console format and changed log level from warning to debug for changelog fetching operations
- Added full metadata, changelogs, and images for Norwegian (`no`) and Portuguese (`pt-PT`). - Ensured consistent markdown formatting in new changelogs.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant update to the application, encompassing numerous enhancements across various features. Key changes include a complete overhaul of the synchronization system for improved reliability, speed, and real-time status updates, alongside support for multi-device and bidirectional background syncing. Task management has been expanded with subtask support, improved quick-add functionality, filtering options for completed tasks and subtasks, task sorting, and optional estimated times. Habit tracking sees improvements in filtering, streak tracking, and time logging. The user interface and experience have been refined with an enhanced app bar, search field, standardized icon sizes, responsive dialogs, a dense view option, and a new multi-page onboarding tour with language selection. Data management is bolstered by new import/export functionalities, a data migration service, and fixes for data loss during updates. Performance and stability are addressed through faster duplicate removal, optimized connections, and improved app startup. Additionally, the update adds support for new locales (cs, da, el, fi, nl, no, pt, ro, sl, sv), improves notification handling, introduces Digital Wellbeing-compatible app usage tracking, and integrates dynamic theming with light mode support. Review comments highlight critical localization issues in the Danish and Czech changelog files, specifically noting that Danish changelogs contain Norwegian text or are entirely in English, and Czech changelogs are in English, requiring proper translation. A minor issue of trailing whitespace in a Czech changelog entry was also identified.
| - Beregnet tid for opgaver er nu virkelig valgfri - du kan la det stå tomt uden at det som standard settes til null | ||
| - Lagt til mulighet for å sette standard beregnet tid for nye opgaver i innstillinger | ||
| - Forbedret håndtering av Windows brannmur for bedre app-funksjonalitet og deteksjon | ||
| - Forbedret integrasjon av tidssporing med vaner og tagger for bedre innsikt | ||
| - Ulike ytelsesforbedringer og feilrettinger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Add markdown editor tooltips for image, heading, and checkbox | ||
| - Remove edit icons and improve spacing in detail forms | ||
| - Add support for cs, da, el, fi, nl, no, pt, ro, sl, sv locales | ||
| - Improve notification scheduling with better validation and fallback handling | ||
| - Implement Windows audio player and add sound assets No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Opravena chyba, kdy mohlo běžet současně více instancí aplikace | ||
| - Vylepšeny indikátory stavu synchronizace a zpětná vazba | ||
| - Opraveny problémy se zobrazováním v ne-anglických jazycích | ||
| - Vylepšena synchronizace sledování využití aplikací a návyků |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Add markdown editor tooltips for image, heading, and checkbox | ||
| - Remove edit icons and improve spacing in detail forms | ||
| - Add support for cs, da, el, fi, nl, no, pt, ro, sl, sv locales | ||
| - Improve notification scheduling with better validation and fallback handling | ||
| - Implement Windows audio player and add sound assets No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Improve logging level from debug to warning for network exceptions - Translate Danish changelog files (da/51.txt, da/65.txt) from English/Norwegian to Danish - Translate Czech changelog file (cs/51.txt) from English to Czech - Remove trailing whitespace from cs/52.txt Several code quality issues were already addressed in previous commits: - Version tracking logic to prevent changelog showing on every startup - Error logging for exception handling in changelog dialog service - Dependency injection refactoring for ChangelogDialog component - Norwegian locale mapping in changelog service Closes #154
- Improve version comparison to handle semantic versioning (split on '+') - Make getCoreVersion public for better testability - Add comprehensive unit tests for changelog services and dialog component - Tests for version parsing, locale mapping, and dialog display - Tests for error handling and edge cases - Add simple in-memory caching to prevent repeated network calls - Cache both successful and failed requests to avoid unnecessary retries Test Coverage Added: - changelog_service_test.dart: Tests network fetching and locale handling - changelog_dialog_service_test.dart: Tests version tracking logic - changelog_dialog_test.dart: Tests UI component rendering and interaction Closes #154
Add mock implementations for changelog-related services and improve test coverage for changelog functionality. Includes updates to test assertions and error handling verification.
- Fixed logger import path from core/shared/utils/logger.dart to core/domain/shared/utils/logger.dart - Updated app_initialization_service.dart import statements after branch merge - Resolved conflicts in changelog dialog components and services
🚀 Motivation and Context
📋 Checklist for Reviewer
Summary by Sourcery
Introduce a localized changelog dialog that is shown during app initialization for new versions, backed by a GitHub-powered changelog service and version-tracking settings.
New Features:
Enhancements:
Build:
Documentation:
Chores: