Skip to content

Not working#346

Closed
omeritzics wants to merge 12 commits into
mainfrom
not-working
Closed

Not working#346
omeritzics wants to merge 12 commits into
mainfrom
not-working

Conversation

@omeritzics
Copy link
Copy Markdown
Owner

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Caution

Review failed

The head commit changed during the review from 3c77f1a to 3bae4ef.

✨ Finishing Touches
🧪 Generate unit tests (beta)

❌ Error committing Unit Tests locally.

  • Create PR with unit tests
  • Commit unit tests in branch not-working
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch not-working

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Note

Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a 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 several enhancements, including displaying APK file sizes on the app page, adding a dialog to pre-fill package names from installed apps, and improving F-Droid metadata fetching with a web-scraping fallback. It also updates various dependencies and documentation. The review feedback highlights critical issues: removing the !hostChanged check in F-Droid metadata fetching can cause incorrect data for custom hosts; the installed apps dialog lacks updateUrlInput: true to trigger a UI rebuild and uses a scrollable configuration that degrades performance; and the app page needs to handle appId changes in didUpdateWidget to prevent displaying stale file sizes.

I am having trouble creating individual review comments. Click here to see my feedback.

lib/app_sources/fdroid.dart (81-151)

high

By removing the !hostChanged check, the code will now attempt to fetch metadata from the official F-Droid GitLab repository even for custom/third-party F-Droid hosts. This can result in displaying incorrect author or changelog information if a third-party app shares the same package ID as an official F-Droid app.

To prevent this correctness issue, the GitLab metadata fetch should be guarded by !hostChanged, falling back to the web scraping logic if it fails or if a custom host is used.

    // Try to fetch author from GitLab metadata (only for official F-Droid)
    bool gitLabSuccess = false;
    if (!hostChanged) {
      try {
        var res = await sourceRequest(
          'https://gitlab.com/fdroid/fdroiddata/-/raw/master/metadata/$appId.yml',
          additionalSettings,
        );
        var lines = res.body.split('\n');
        var authorLines = lines.where((l) => l.startsWith('AuthorName: '));
        if (authorLines.isNotEmpty) {
          details.names.author = authorLines.first
              .split(': ')
              .sublist(1)
              .join(': ');
        }
        var changelogUrls = lines
            .where((l) => l.startsWith('Changelog: '))
            .map((e) => e.split(' ').sublist(1).join(' '));
        if (changelogUrls.isNotEmpty) {
          details.changeLog = changelogUrls.first;
          bool isGitHub = false;
          bool isGitLab = false;
          try {
            GitHub(
              hostChanged: true,
            ).sourceSpecificStandardizeURL(details.changeLog!);
            isGitHub = true;
          } catch (e) {
            //
          }
          try {
            GitLab(
              hostChanged: true,
            ).sourceSpecificStandardizeURL(details.changeLog!);
            isGitLab = true;
          } catch (e) {
            //
          }
          if ((isGitHub || isGitLab) &&
              (details.changeLog?.indexOf('/blob/') ?? -1) >= 0) {
            try {
              details.changeLog = (await sourceRequest(
                details.changeLog!.replaceFirst('/blob/', '/raw/'),
                additionalSettings,
              )).body;
            } catch (e) {
              // Fail silently
            }
          }
        }
        if ((details.changeLog?.length ?? 0) > 2048) {
          details.changeLog = '${details.changeLog!.substring(0, 2048)}...';
        }
        gitLabSuccess = true;
      } catch (e) {
        // Fail silently, fallback to web page scraping
      }
    }

    if (!gitLabSuccess) {
      // If GitLab metadata fails or host is custom, try to fetch author from F-Droid web page
      try {
        var res = await sourceRequest(
          'https://$host/packages/$appId',
          additionalSettings,
        );
        var body = parse(res.body);
        var authorElement = body.querySelector('a[href^="mailto:"]');
        if (authorElement != null) {
          var authorText = authorElement.text.trim();
          if (authorText.isNotEmpty) {
            details.names.author = authorText;
          }
        }
      } catch (e) {
        // Fail silently, keep fallback author
      }
    }

lib/pages/add_app.dart (909-913)

high

The changeUserInput function is called here without passing updateUrlInput: true. Because of this, urlInputKey is not incremented, and the GeneratedForm (which relies on urlInputKey as its key) will not rebuild. As a result, the text field in the UI will not actually update to show the pre-filled package name when an app is tapped.

Pass updateUrlInput: true to ensure the form updates correctly.

                                changeUserInput(
                                  app.packageName ?? '',
                                  true,
                                  false,
                                  updateUrlInput: true,
                                );

lib/pages/add_app.dart (882-891)

medium

Using scrollable: true on the AlertDialog wraps its content in a SingleChildScrollView. When a ListView.builder with shrinkWrap: true is placed inside it without a bounded height, it forces the ListView to layout all its items at once. For users with many installed apps, this defeats the lazy-loading benefits of ListView.builder and triggers hundreds of asynchronous platform channel calls simultaneously, causing severe UI lag.

Instead, keep scrollable: false (default) on the dialog, and wrap the ListView.builder in a SizedBox with a bounded height (e.g., 60% of the screen height) to enable efficient lazy-loading.

                return AlertDialog(
                  contentPadding: const EdgeInsets.fromLTRB(24, 16, 24, 16),
                  title: Text(t('installedApps')),
                  content: SizedBox(
                    width: double.maxFinite,
                    height: MediaQuery.of(ctx).size.height * 0.6,
                    child: ListView.builder(
                      itemCount: installedApps.length,
                      itemBuilder: (context, index) {

lib/pages/app.dart (58-65)

medium

The file size is fetched in initState, but if the parent widget rebuilds with a different appId (for example, in a master-detail layout), the state is reused and initState is not called again. This would cause the page to display the file size of the previously loaded app.

Implement didUpdateWidget to clear the old file size and trigger a new fetch when widget.appId changes.

  @override
  void initState() {
    super.initState();
    // Fetch APK file size when the app is loaded
    WidgetsBinding.instance.addPostFrameCallback((_) {
      _fetchApkFileSize();
    });
  }

  @override
  void didUpdateWidget(AppPage oldWidget) {
    super.didUpdateWidget(oldWidget);
    if (oldWidget.appId != widget.appId) {
      setState(() {
        _apkFileSize = null;
      });
      _fetchApkFileSize();
    }
  }

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

No files have been changed in this PR. Unable to generate unit tests.

@omeritzics
Copy link
Copy Markdown
Owner Author

@gemini-code-assist Will merging this PR harm the project? Say yes or no.

@gemini-code-assist
Copy link
Copy Markdown

Note

Gemini is unable to generate a issue-comment-reply for this pull request due to the file types involved not being currently supported.

@omeritzics omeritzics closed this May 26, 2026
@omeritzics omeritzics deleted the not-working branch May 26, 2026 14:38
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.

1 participant