Skip to content

feat: ads module#3215

Open
ice-deimos wants to merge 73 commits into
masterfrom
feat/ads-module
Open

feat: ads module#3215
ice-deimos wants to merge 73 commits into
masterfrom
feat/ads-module

Conversation

@ice-deimos

Copy link
Copy Markdown
Contributor

Description

Advertising package for ICE

Additional Notes

Used to show native ad in different places in application

Task ID

4167

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Documentation
  • Chore

Screenshots (if applicable)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a comprehensive advertising module (ion_ads) to integrate Appodeal native ads throughout the ICE application. The module provides platform abstractions, UI components, and integrations across multiple app features including feed, stories, chat, and articles.

Changes:

  • New ion_ads package with platform abstraction layer and UI widgets for native ads
  • Integration of ads in feed, stories, chat list, and article views
  • Android and iOS platform-specific implementations for Appodeal SDK
  • Environment configuration for ad keys and initialization

Reviewed changes

Copilot reviewed 201 out of 241 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
packages/ion_ads/pubspec.yaml Package definition for the new ads module
packages/ion_ads/lib/src/widgets/* Native ad UI components for different placements
packages/ion_ads/lib/src/services/* Platform abstraction and ad insertion logic
packages/ion_ads/lib/src/models/* Data models for ad assets and types
packages/ion_ads/android/* Android-specific Appodeal SDK implementation
packages/ion_ads/ios/* iOS-specific Appodeal SDK implementation
lib/app/services/ion_ad/* Main app integration provider
lib/app/features/feed/* Feed integration with ad cards
lib/app/features/chat/* Chat list ad integration
android/build.gradle Appodeal SDK dependencies
pubspec.yaml Added ion_ads dependency
Files not reviewed (2)
  • packages/ion_ads/example/ios/Runner.xcodeproj/project.xcworkspace/contents.xcworkspacedata: Language not supported
  • packages/ion_ads/example/ios/Runner.xcworkspace/contents.xcworkspacedata: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/ion_ads/lib/src/widgets/native_article_ad.dart Outdated
Comment thread packages/ion_ads/lib/src/widgets/native_video_ad.dart Outdated
Comment thread packages/ion_ads/test/ion_ad_test.dart
Comment thread packages/ion_ads/lib/src/services/ad_insertion_helper.dart
Comment thread packages/ion_ads/lib/src/widgets/native_ad_card.dart Outdated
Comment thread packages/ion_ads/lib/src/components/call_to_action_button.dart Outdated
Comment thread packages/ion_ads/lib/src/widgets/native_chat_list_ad.dart Outdated
Comment thread packages/ion_ads/lib/src/widgets/native_chat_ad.dart
@ice-deimos ice-deimos marked this pull request as ready for review January 29, 2026 13:45
@ice-morpheus ice-morpheus enabled auto-merge (squash) January 29, 2026 13:45
@ice-morpheus ice-morpheus force-pushed the feat/ads-module branch 3 times, most recently from 71901e8 to 129eaa3 Compare January 30, 2026 08:56
@github-actions

Copy link
Copy Markdown

🤖 Gemini 2.5 Flash Code Review

As a Senior Software Engineer, I've reviewed the provided code changes. Here's my feedback focusing on potential bugs, security vulnerabilities, and readability.


Overall Summary

This Pull Request primarily introduces ad integration (Appodeal, AdMob) across both Android and iOS platforms, along with a new GitHub Actions workflow for AI code review. While the ad integration aims to monetize, it introduces several significant security and privacy concerns, especially on iOS, and some potential logic issues for ad display.

1. Potential Bugs or Logic Errors

  • Android AdMob ID Configuration (android/app/build.gradle, AndroidManifest.xml):
    • Ensure dartEnv['ADMOB_ID_ANDROID'] is reliably defined and passed through your Flutter environment configuration. If this value is empty or invalid at build time, AdMob initialization will fail silently or crash.
  • iOS Pod Dependency Bloat (ios/Podfile):
    • The appodeal function adds an extensive list of mediation adapters. Review if all these adapters are truly necessary for your advertising strategy. Including only required adapters reduces app size, build times, and potential dependency conflicts.
  • Chat Ad Injection Logic (lib/app/features/chat/providers/conversation_messages_provider.r.dart):
    • Ad Content Mismatch: The _injectAdIfPossible method creates an EventMessage for the ad, copying the content and createdAt from the lastMessageItem. However, the TextMessage widget then completely ignores this copied content and renders an AppodealNativeAd. This is inconsistent and potentially confusing. The ad EventMessage should be a minimalist placeholder, or its copied content should be meaningful to ad rendering.
    • Ad Placement: Confirm that inserting the ad message at tail.insert(0, adMessageItem) (which means before the newest real message in that segment) is the desired placement in the chat UI.
  • Recent Chats Ad Injection Logic (lib/app/features/chat/recent_chats/views/pages/recent_chats_timeline_page/recent_chats_timeline_page.dart):
    • Edge Case for rng.nextInt: The line final adIndex = rng.nextInt(conversations.length - 1); will throw an ArgumentError if conversations.length is 1 or less. Add a check like if (conversations.length < 2) return; before calling rng.nextInt.
    • Misleading Ad UI: The ad ConversationListItem is created by copying the lastConversation. This means the ad entry in the recent chats list will display the name, avatar, and other metadata of a real conversation. This is misleading to users. Ad items should have distinct, ad-specific UI and content.
  • Fixed Heights for Native Ads (text_editor_ad_block.dart, recent_chats_timeline_page.dart):
    • Using fixed heights (e.g., height: 286 in TextEditorAdBuilder, height: 72.0.s in AdChatTile) for native ads might lead to content clipping, excessive padding, or layout issues. Native ads are often dynamic; consider making their containers adaptive or providing minimum/maximum constraints instead.
  • ConversationType.ad Usage: While ConversationType.ad is added to the enum, the injected EventMessage in conversation_messages_provider.r.dart still uses the kind of the last message, not explicitly the new ad type. Although the UI relies on ID prefixes, aligning the data model with the intent would be cleaner.

2. Security Vulnerabilities

  • CRITICAL: NSAllowsArbitraryLoads = <true/> (iOS Info.plist):
    • This is a major security vulnerability. Setting NSAllowsArbitraryLoads to true completely disables Apple's App Transport Security (ATS) for all network connections. This allows unencrypted HTTP traffic, making the app vulnerable to eavesdropping and man-in-the-middle attacks. This should be avoided. If specific ad networks require cleartext, use NSExceptionDomains to whitelist only those domains, rather than disabling ATS globally. This could also lead to app rejection during Apple review.
  • High: NSCalendarsUsageDescription (iOS Info.plist):
    • Requesting calendar access for "personalised advertising" is highly intrusive and raises significant privacy concerns. This is very likely to cause user distrust and could lead to app rejection by Apple. Re-evaluate if this permission is truly indispensable for your ad experience. If so, provide an extremely clear and compelling justification to users and Apple.
  • Privacy Implications (AD_ID, NSUserTrackingUsageDescription, NSLocationWhenInUseUsageDescription):
    • The changes (enabling AD_ID permission on Android, adding NSUserTrackingUsageDescription and NSLocationWhenInUseUsageDescription on iOS) indicate that your app will now collect advertising identifiers and potentially location data for ad tracking. This necessitates strict adherence to global privacy regulations (GDPR, CCPA) and Apple's App Tracking Transparency (ATT) framework. Users must be clearly informed and give explicit consent via the ATT prompt on iOS.
  • Third-Party Repository Trust (android/build.gradle, ios/Podfile):
    • Adding external Maven and CocoaPods repositories (Appodeal, Verve, Bidon) introduces trust dependencies. Ensure these repositories are legitimate and have good security practices, as a compromised repository could inject malicious code.

3. Code Readability and Style Improvements

  • GitHub Workflow - Newline (.github/workflows/gemini_workflow.yml):
    • Add a newline character at the end of the file for better git diff behavior and general file formatting best practices.
  • Custom Ad Block License (lib/app/components/text_editor/components/custom_blocks/text_editor_ad_block.dart):
    • Clarify the SPDX-License-Identifier ice License 1.0. Is this a custom license, or a typo (e.g., MIT, Apache-2.0)?
  • Comments for Ad Injection Logic:
    • Add comments to the _injectAdIfPossible methods in both conversation_messages_provider.r.dart and recent_chats_timeline_page.dart to clearly explain the rationale behind copying content/metadata from other messages and the exact intended placement of the injected ad items.

Recommendation:

Prioritize addressing the critical security vulnerabilities (NSAllowsArbitraryLoads = <true/>, NSCalendarsUsageDescription) immediately. Then, refine the ad injection logic to prevent misleading user experiences and ensure optimal performance and layout for native ads. Conduct a thorough privacy policy update and ensure compliance with all relevant regulations for data collection.

@github-actions

Copy link
Copy Markdown

🤖 Gemini Flutter & Code Review

As a Senior Software Engineer, I've reviewed the provided code changes. This PR introduces a substantial new feature: ad integration across various parts of the application (chat, feed, stories, video pages) using Appodeal. The changes span multiple platforms (Android, iOS) and Dart code, involving new UI components, business logic for ad placement, and infrastructure setup for the ad SDK.

Overall, the implementation appears well-structured, leveraging Flutter's widget tree and Riverpod for state management. The new ion_ads package is a good approach for encapsulating platform-specific ad SDK integration.

Here are my findings and recommendations:


🚨 Critical Issues

  1. lib/app/services/ion_ad/ion_ad_provider.r.dart - Hardcoded hasConsent: true

    • Issue: The AppodealIonAdsPlatform.initialize method is called with hasConsent: true unconditionally. This is a critical privacy and compliance concern, especially for regions with strict data protection laws like GDPR (Europe) and CCPA (California). Users must be prompted for consent before personal data is used for advertising, and their choice must be respected. The AppodealConsentManager in the native plugin indicates consent management is supported, but it's not integrated here.
    • Recommendation: This needs immediate attention. The hasConsent parameter must dynamically reflect the user's actual consent status, which should be obtained and persisted using a consent management platform (CMP), ideally integrated with Appodeal's Consent Manager.
  2. packages/ion_ads/android/src/main/kotlin/com/appodeal/appodeal_flutter/IonAppodealFlutterPlugin.kt - Production Logging

    • Issue: The apdLog function uses if (true) Log.d(...). This means that debug logs from the Appodeal Flutter plugin will always be enabled, even in release builds. This can lead to excessive log output in production, potentially exposing sensitive information, impacting performance, and increasing battery drain.
    • Recommendation: Change the condition to if (BuildConfig.DEBUG) or similar (e.g., a configurable flag) to ensure these logs are only active in debug builds.

⚠️ Potential Issues & Suggestions

  1. ios/Podfile - Extensive Ad Adapters

    • Observation: A very long list of Appodeal adapter pods (e.g., APDAppLovinAdapter, APDBidMachineAdapter, AppLovinMediationAmazonAdMarketplaceAdapter, etc.) has been added.
    • Suggestion: While this might be necessary for broad mediation, it would be beneficial to add a high-level comment explaining why so many adapters are included, or if there's a strategy to selectively include them (e.g., based on regions or desired ad networks) to minimize the app size and build complexity.
  2. Ad Placement Randomness (Minor)

    • lib/app/features/chat/recent_chats/views/pages/recent_chats_timeline_page/recent_chats_timeline_page.dart: The Random generator for adIndex is seeded with conversations.length. If the conversation list often has the same length, the ad might consistently appear at the same index across different app sessions or users.
    • lib/app/features/feed/stories/providers/user_stories_provider.r.dart: Similarly, Random(storiesCount) is used.
    • Recommendation: For more varied ad placement, consider seeding the Random generator with a more dynamic value, such as DateTime.now().millisecondsSinceEpoch (as already used in parsed_media_provider.r.dart). This ensures a different ad position each time the list is loaded.
  3. Ad Injection Logic in videos_vertical_scroll_page.dart - prevVideoIndex Calculation (Minor)

    • Observation: In itemBuilder, when rendering AdVideoViewer, flattenedVideos[prevVideoIndex] is used. prevVideoIndex is calculated using max(0, index - adPositions.where((p) => p < index).length - 1).
    • Suggestion: Ensure flattenedVideos is not empty when adPositions.contains(index) is true, as prevVideoIndex could theoretically still point to an invalid index if flattenedVideos is very small and index is 0 or 1. The totalItemCount calculation and adPositions generation logic should ideally prevent this, but it's a point to double-check.

✅ Good Practices & Positive Observations

  • GitHub Workflow (.github/workflows/gemini_workflow.yml): Excellent to see automated code reviews being introduced with Gemini AI. This is a valuable addition to the CI/CD pipeline.
  • Environment Variable Management: Ad keys (AD_APP_KEY_ANDROID, AD_APP_KEY_IOS, ADMOB_ID_ANDROID) are correctly moved to environment variables and accessed via EnviedField and envProvider. This prevents hardcoding sensitive information and promotes secure configuration.
  • Modular Ad Integration: The creation of a dedicated ion_ads Flutter plugin for Appodeal is a great architectural choice. It encapsulates platform-specific code and keeps the main application cleaner.
  • Clear Separation of Concerns:
    • Ad-related UI (TextEditorAdEmbed, TextEditorAdBuilder, AdChatTile, _CustomNativeAd, AdStoryViewer, AdVideoViewer) is well-defined as separate widgets.
    • Ad injection logic is contained within providers (ConversationMessages, parsed_media_provider, UserStories, FeedPostsProvider, VideosVerticalScrollPage), ensuring business logic is separate from UI.
  • Null Safety and Error Handling: firstOrNull, ??, and try-catch blocks are used appropriately, minimizing the risk of null-pointer exceptions and handling potential issues during ad insertion gracefully. No force unwrapping (!) found, which adheres to best practices.
  • Performance Optimizations: Use of const constructors, useMemoized, and ValueKey where appropriate contributes to better performance and reduced widget rebuilds. useAnimationController with keys ensures proper resource management.
  • Ad Inclusion Across Features: The integration covers various content types (text editor, chat, feed posts, stories, videos), showing a comprehensive ad strategy.
  • SKAdNetworkItems in ios/Runner/Info.plist: Inclusion of the vast list of SKAdNetworkIdentifiers is correct for iOS ad tracking. NSUserTrackingUsageDescription is also correctly added for ATT compliance.

Summary of Actionable Feedback:

  1. Prioritize Fixing Consent Management: Implement proper user consent flow for advertising in lib/app/services/ion_ad/ion_ad_provider.r.dart instead of hardcoding hasConsent: true.
  2. Disable Debug Logging in Production: Modify apdLog in IonAppodealFlutterPlugin.kt to only log in debug builds.
  3. Add Comment for iOS Pods: Explain the necessity of numerous ad adapter pods in ios/Podfile.

Overall, this is a significant and well-implemented feature, with the critical consent management point being the main area requiring immediate attention.

@github-actions

Copy link
Copy Markdown

🤖 Gemini Flutter & Code Review

As a Senior Software Engineer, I've reviewed the provided code changes with a focus on Flutter/Mobile best practices and general code quality.

Here's my feedback:

🛡️ General Code Quality

  • Security & Privacy (Critical - lib/app/services/ion_ad/ion_ad_provider.r.dart)

    • Issue: The AppodealIonAdsPlatform.initialize method is called with hasConsent: true unconditionally. This is a critical privacy and compliance violation (e.g., GDPR, CCPA, ePrivacy Directive) if the app operates in regions requiring user consent for tracking. User consent must be collected and respected before initializing the Appodeal SDK with hasConsent: true or serving personalized ads.
    • Recommendation: Implement a Consent Management Platform (CMP) (e.g., Google's User Messaging Platform (UMP) SDK, OneTrust, TrustArc) to manage and collect user consent. The hasConsent parameter should dynamically reflect the user's actual consent status retrieved from the CMP.
  • Architecture / Separation of Concerns (Minor - Multiple Files)

    • Issue: Ads are injected into existing data lists (List<EventMessage>, List<ConversationListItem>, List<ModifiablePostEntity>) by creating "fake" data model items (e.g., EventMessage with adIdPrefix, ConversationListItem with ConversationType.ad). While pragmatic for reusing existing list rendering logic, this blurs the line between core domain data and UI-specific ad placeholders.
    • Recommendation: For improved clarity and type safety, consider introducing a distinct "mixed content" data model (e.g., a sealed class or union type) that explicitly represents both genuine content items and ad items. This makes the nature of each list element explicit. For this PR, the use of distinct prefixes (adIdPrefix, adKeySuffix) mitigates immediate confusion, but it's a point for future architectural refinement.

📱 Flutter & Mobile Analysis

  • iOS Info.plist - Location Usage (Minor - ios/Runner/Info.plist)

    • Issue: NSLocationWhenInUseUsageDescription has been added stating location is used for "analytics and advertising purposes".
    • Recommendation: Verify if precise location access is strictly necessary for these purposes. If broader location data (e.g., region derived from IP) or no location data suffices, consider using a less intrusive permission or removing the request to enhance user privacy and reduce permission fatigue.
  • Performance / Native Ad Layouts (Minor - packages/ion_ads/android/... Native UI)

    • Issue: The native Android ad views and their dynamic layout adjustments in TemplateNativeAdViewBinderImpl are complex.
    • Recommendation: Conduct thorough UI/UX testing of these native ad layouts across a diverse range of Android devices, screen sizes, and API levels. Ensure ads render consistently, gracefully handle varying content (e.g., long titles, no image), and do not cause performance regressions or visual glitches.
  • Randomness Seeding (Minor - lib/app/features/chat/recent_chats/..., lib/app/features/feed/stories/..., lib/app/features/video/...)

    • Issue: Random instances are sometimes seeded with values like list.length or storiesCount. If these values remain constant across app sessions, the sequence of "random" ad placements will be predictable.
    • Recommendation: For a more truly random and varied ad distribution experience across different app sessions, seed Random instances with a more volatile value like DateTime.now().millisecondsSinceEpoch. If consistent placement within a session for given content is the goal, the current approach is acceptable.

File-Specific Notes

  • ios/Podfile: The extensive list of Appodeal and mediation network pods is expected for a robust ad integration. Grouping them within the appodeal function is good practice. The warn_for_multiple_pod_sources: false is an appropriate adjustment given the added sources.
  • lib/app/components/text_editor/components/custom_blocks/text_editor_ad_block.dart: The use of const for the ad key and correct implementation of CustomBlockEmbed and EmbedBuilder are good.
  • lib/app/features/chat/providers/conversation_messages_provider.r.dart: Using adIdPrefix and adKeyPrefix for "fake" ad messages is a clever workaround. The logic to not show ads after messages from the current user is a good UX consideration.
  • lib/app/features/feed/providers/parsed_media_provider.r.dart: Conditional ad injection only for ArticleData and when native ads are loaded is well-handled. Inserting \n before the ad embed ensures proper layout.
  • lib/app/features/feed/stories/views/components/story_viewer/components/viewers/ad_story_viewer.dart: Using IgnorePointer for the AppodealNativeAd within the story viewer is critical to ensure story navigation gestures are not blocked by the ad content. The useOnInit to mark the ad as loaded for story progression is a good detail.
  • lib/app/features/video/views/pages/ad_video_page.dart: The structured approach with a loading indicator and a progress bar for video ads provides a good user experience. Correctly handling topMargin for the ad choice config is also a good detail.
  • lib/app/features/core/providers/env_provider.r.dart: Externalizing ad-related keys (AD_APP_KEY_ANDROID, AD_APP_KEY_IOS, ADMOB_ID_ANDROID) into environment variables is excellent for security and configuration management.
  • packages/ion_ads/: The new Flutter package appears to be a comprehensive and well-structured wrapper around the Appodeal SDK, providing extensive configuration options for native ads. This is a significant piece of work.

Overall, the PR introduces a comprehensive ad integration. The primary concern is the critical privacy issue regarding user consent for advertising. Once that is addressed, the remaining points are minor refinements.

@ice-deimos ice-deimos force-pushed the feat/ads-module branch 2 times, most recently from 19d6bf0 to 87c0612 Compare February 6, 2026 08:48
@github-actions

github-actions Bot commented Feb 6, 2026

Copy link
Copy Markdown

🤖 Gemini Flutter & Code Review

Pull Request Review: Ad Integration

This Pull Request introduces significant changes related to integrating an advertising SDK (Appodeal) across various parts of the mobile application, including chat, articles, stories, and video feeds. The overall architecture, especially the creation of a dedicated Flutter plugin (ion_ads), is a robust approach to encapsulate platform-specific logic.

Here are the review comments:


🛡️ General Code Quality

  • Architectural Consistency: The decision to create a new Flutter plugin (packages/ion_ads) for Appodeal integration is excellent. It ensures a clear separation of concerns, keeping the main application code clean from native SDK specifics and improving maintainability.

📱 Flutter & Mobile Analysis

ios/Podfile

  • Suggestion (Maintainability): The appodeal function in the Podfile lists a very extensive set of mediation adapters with specific versions. While necessary for broad ad network support, this list can become challenging to maintain.
    • Recommendation: Regularly review if all listed mediation adapters are actively used and beneficial. If Appodeal offers a more consolidated way to include adapters (e.g., umbrella pods for common networks), consider adopting it to reduce the number of explicit pod declarations. This is more of a strategic/maintenance suggestion than an immediate issue.

lib/app/features/chat/providers/conversation_messages_provider.r.dart

  • Logic Review (Ad Placement): The _injectAdIfPossible method inserts an ad EventMessage at index 0 (the newest position) of the tail list.
    • Observation: If the conversation tail is frequently updated (e.g., when new messages arrive or on screen refresh), and the conditions for ad injection are met, an ad will consistently appear as the very first message. This could lead to a disruptive user experience with ads appearing too often at the top of the chat or shifting content unexpectedly.
    • Recommendation: Consider a more sophisticated ad placement strategy for chat messages:
      • Fixed Interval: Inject ads only after a certain number of non-ad messages (e.g., every 5th or 10th message).
      • Scroll-Driven: Inject ads dynamically as the user scrolls, similar to how they are handled in the feed_posts_list or videos_vertical_scroll_page.
      • Contextual: Base placement on message timestamps or other contextual factors to ensure a natural flow.
  • Code Style & Best Practices (Separation of Concerns): The _injectAdIfPossible method directly constructs an EventMessage to represent an ad.
    • Recommendation: Extract the logic for creating ad-specific EventMessage objects into a dedicated factory or helper class (e.g., AdMessageFactory). This would keep the ConversationMessages provider focused solely on managing conversation data and prevent it from becoming tightly coupled with ad-specific data modeling.

lib/app/features/chat/recent_chats/views/pages/recent_chats_timeline_page/recent_chats_timeline_page.dart

  • Performance/Logic (Randomness for Ad Index): The Random(conversations.length) is used for ad placement.
    • Observation: If conversations.length is static or consistent across app launches for a given user, the ad might always appear at the same index.
    • Recommendation: While useOnInit makes this a one-time placement, using Random(DateTime.now().millisecondsSinceEpoch) (as seen in parsed_media_provider.r.dart) would provide more varied ad placement across app launches, improving the perceived freshness of ads.

lib/app/services/ion_ad/ion_ad_provider.r.dart

  • Security & Privacy (Consent Management): The platform.initialize call uses hasConsent: true.
    • Critical Concern: For compliance with privacy regulations like GDPR, CCPA, etc., user consent for tracking and personalized advertising must be explicitly obtained before initializing ad SDKs with hasConsent: true. Assuming consent could lead to legal and privacy issues.
    • Recommendation: Thoroughly review the app's privacy policy and consent management flow. Ensure that actual user consent is collected and the hasConsent parameter is dynamically set based on the user's explicit choices. If the app does not collect personal data for ads, hasConsent: false might be more appropriate by default.

lib/app/features/video/views/pages/videos_vertical_scroll_page.dart

  • Code Style (Magic Numbers): The ad placement logic in _loadMore for video scrolls uses a formula with several implicit numbers: showAdAfter = 7 and the gap calculation (showAdAfter + rng.nextInt(showAdAfter) - showAdAfter / 2).toInt() with a minimum of 3.
    • Recommendation: While the intent is clear, consider defining showAdAfter (which is already a constant, good!), and potentially minAdInterval and maxAdIntervalDeviation as named constants. This would improve readability and make it easier to adjust ad frequency.

Conclusion

The ad integration is well-designed with the use of a dedicated Flutter plugin and custom native ad layouts, which is highly commendable. The adoption of EnviedField for API keys is also a strong point for security. The primary areas for attention are the nuanced ad placement strategies in various parts of the app for optimal user experience and, most importantly, ensuring robust privacy compliance regarding user consent for advertising.

@github-actions

github-actions Bot commented Feb 7, 2026

Copy link
Copy Markdown

🤖 Gemini Flutter & Code Review

Acting as a Senior Software Engineer, I've reviewed the provided code changes.

🛡️ General Code Quality

  • .github/workflows/gemini_workflow.yml
    • New Workflow: This new workflow for Gemini AI Code Review is well-defined. Permissions are appropriate, dependencies are installed correctly, and secrets are handled securely through GitHub Actions secrets.
    • Verdict: Looks good.

📱 Flutter & Mobile Analysis

android/app/build.gradle

  • AdMob ID Configuration: The addition of admob_id to manifestPlaceholders for both release and staging builds, pulling from dartEnv['ADMOB_ID_ANDROID'], is a good practice for environment-specific configuration and avoiding hardcoding.
  • Appodeal SDK Dependencies: The inclusion of Appodeal SDK and its exclusion of specific network modules is standard for ad network integration.
  • Verdict: Looks good.

android/app/src/main/AndroidManifest.xml

  • AD_ID Permission: The change to com.google.android.gms.permission.AD_ID from remove to replace is necessary for ad SDKs on Android 13+. The added comment clarifies its purpose.
  • AdMob Application ID: The meta-data tag for com.google.android.gms.ads.APPLICATION_ID using ${admob_id} is correctly placed and sourced.
  • Verdict: Looks good.

android/build.gradle

  • Appodeal Repositories: The addition of Appodeal and Verve Maven repositories to allprojects.repositories is required for the new ad SDK dependencies.
  • Verdict: Looks good.

ios/Podfile

  • Appodeal Pods: The integration of Appodeal pods and a wide range of mediation adapters (Amazon, AppLovin, GoogleAdMob, etc.) within a dedicated appodeal function in the Runner target is a clean and maintainable way to manage these dependencies.
  • Multiple Pod Sources: The install! 'cocoapods', :warn_for_multiple_pod_sources => false is a common fix to prevent warnings when using multiple private spec repositories.
  • Verdict: Looks good.

ios/Runner/Info.plist

  • Privacy & Ad Identifiers:
    • NSUserTrackingUsageDescription is crucial for App Tracking Transparency (ATT) on iOS 14.5+ and is well-defined.
    • GADApplicationIdentifier is correctly set to $(ADMOB_ID).
    • NSLocationWhenInUseUsageDescription provides necessary context for location-based advertising/analytics.
    • The extensive list of SKAdNetworkItems is important for ad attribution on iOS, especially when ATT consent is not granted.
  • Verdict: Looks good.

lib/app/components/text_editor/components/custom_blocks/text_editor_ad_block.dart

  • New Ad Block: The TextEditorAdEmbed and TextEditorAdBuilder provide a clean mechanism to integrate native ads within the Quill text editor.
  • Performance Optimization: The ad block uses a fixed height: 286, which is efficient as it avoids complex layout calculations for the ad widget.
  • Code Style & Best Practices: Named constants like textEditorAdKey are used, and class/method names are clear.
  • Verdict: Looks good.

lib/app/features/chat/model/database/tables/conversation_table.dart

  • New ConversationType: Adding ad to the ConversationType enum clearly categorizes ad entries in the database, improving data modeling.
  • Verdict: Looks good.

lib/app/features/chat/providers/conversation_messages_provider.r.dart

  • Ad Injection Logic: The _injectAdIfPossible method conditionally inserts a synthetic EventMessage representing an ad into the conversation stream. This reuses the existing EventMessage model by leveraging unique adIdPrefix and adKeyPrefix for identification.
  • Code Style & Best Practices: adIdPrefix and adKeyPrefix are correctly defined as static const.
  • Verdict: Looks good.

lib/app/features/chat/recent_chats/views/pages/recent_chats_timeline_page/recent_chats_timeline_page.dart

  • Ad Placement in Recent Chats: The useOnInit hook for injecting an ad into the conversations list is a suitable approach for one-time setup. It ensures ads appear dynamically without altering core data fetching logic. The use of copyWith to create an ad-specific ConversationListItem is effective.
  • AdChatTile Widget: This new widget provides a dedicated UI for ad type conversations, integrating AppodealNativeAd with specific NativeAdType.chat options and a fixed height.
  • Randomness: The Random(conversations.length) seed for ad placement might lead to predictable placement if conversations.length is consistent. Consider Random(DateTime.now().millisecondsSinceEpoch) for more varied ad experiences across app sessions.
  • Verdict: Mostly good. Suggest considering a different seed for the Random generator for more dynamic ad placements.

lib/app/features/chat/views/components/message_items/message_types/text_message/text_message.dart

  • Ad Message Rendering: The isAd check correctly diverts ad EventMessages to the new _AdItem widget.
  • _AdItem Widget: This widget displays AppodealNativeAd and overlays an IONLoadingIndicatorThemed for a better user experience while the ad loads. The fixed dimensions (width: 246, height: 222) for the ad container are efficient.
  • Verdict: Looks good.

lib/app/features/chat/views/pages/conversation_page/conversation_page.dart

  • Ad Conversation Routing: Explicitly routing ConversationType.ad to OneToOneMessagesPage ensures ad messages are handled by the appropriate UI component.
  • Verdict: Looks good.

lib/app/features/core/providers/env_provider.r.dart

  • Environment Variables: The addition of AD_APP_KEY_ANDROID, AD_APP_KEY_IOS, and ADMOB_ID_ANDROID as @EnviedFields is an excellent practice for externalizing sensitive and environment-specific keys.
  • Security & Privacy: This design choice prevents hardcoding of secrets, improving security.
  • Verdict: Excellent.

lib/app/features/core/providers/init_provider.r.dart

  • Ad Client Initialization: Listening to ionAdClientProvider during app initialization ensures the ad SDK is ready early, which is crucial for ad readiness and performance.
  • Verdict: Looks good.

lib/app/features/feed/providers/parsed_media_provider.r.dart

  • Article Ad Injection: Logic to inject TextEditorAdEmbed into ArticleData (Quill Delta format) if native ads are available. This expands ad placement to article content.
  • Performance Optimization: Ad injection is conditional and uses Random(DateTime.now().millisecondsSinceEpoch) for varied ad placement.
  • Verdict: Looks good.

lib/app/features/feed/stories/providers/user_stories_provider.r.dart

  • Story Ad Injection: The _generateAdIndexes method strategically inserts ad placeholders (ModifiablePostEntity with adKeySuffix) into user stories, creating dynamic ad breaks.
  • Randomness: Similar to recent chats, consider using a DateTime.now().millisecondsSinceEpoch based seed for Random in _generateAdIndexes to ensure more varied ad placements across app sessions, rather than storiesCount.
  • Verdict: Mostly good. Suggest considering a different seed for the Random generator for more dynamic ad placements.

lib/app/features/feed/stories/views/components/story_viewer/components/header/story_header_gradient.dart

  • UI Change: The increase in height and switch from LinearGradient to RadialGradient are visual updates. Assuming this aligns with design requirements.
  • Verdict: Looks good (design-dependent).

lib/app/features/feed/stories/views/components/story_viewer/components/progress/story_progress_bar_container.dart

  • Ad Story Count: The max function in storiesCount calculation correctly accounts for dynamically inserted ad stories, ensuring the progress bar reflects the total number of viewable items.
  • Verdict: Looks good.

lib/app/features/feed/stories/views/components/story_viewer/components/viewers/ad_story_viewer.dart

  • Ad Story Viewer: This new widget provides a dedicated fullscreen view for story ads. markLoaded() is correctly called onInit to prevent the viewer from waiting for non-existent story content. It includes a loading indicator and a close button.
  • Verdict: Looks good.

lib/app/features/feed/views/pages/article_details_page/components/article_content_measurer.dart

  • Ad Block Integration: Adding TextEditorAdBuilder to the QuillEditor ensures ads embedded in articles are rendered correctly in the detail view.
  • Verdict: Looks good.

lib/app/features/feed/views/pages/feed_page/components/feed_posts_list/feed_posts_list.dart

  • Feed Ad Insertion: The _getFeedListItems method now dynamically inserts _CustomNativeAd widgets into the feed based on startAdOffset and ionAdClient.computeInsertionIndices.
  • _CustomNativeAd Widget: This stateless widget provides a fixed-height container for AppodealNativeAd, specifically for content streams.
  • Error Handling: The broad try-catch block around initialListItems.insert (on Object catch (_) { // Ignore all errors }) might hide unexpected issues. While it ensures robustness, consider logging the error for debugging or narrowing the exception type if possible.
  • Verdict: Mostly good. Suggest refining the try-catch block for better error visibility.

lib/app/features/video/views/pages/ad_video_page.dart

  • New Ad Video Viewer: The AdVideoViewer provides a dedicated page for video ads, displaying a loading indicator and an AppodealNativeAd.
  • Ad Progress Indicator: The _AdVideoProgress widget is a good addition, using useAnimationController to show a 5-second progress bar for the ad, improving user feedback. useEffect with videoId as a key properly resets the animation for new ads.
  • Verdict: Looks good.

lib/app/features/video/views/pages/videos_vertical_scroll_page.dart

  • Video Feed Ad Injection: Ads are dynamically inserted into the PageView.builder for the video feed using adPositionsRef and useMemoized. The onPageChanged callback handles visibility of app bar actions based on whether an ad is currently displayed.
  • Performance Optimization: useMemoized is correctly used for adPositions calculation, preventing unnecessary re-computations.
  • Verdict: Looks good.

lib/app/services/ion_ad/ion_ad_provider.r.dart

  • New Ad Service Provider: The ionAdClientProvider correctly initializes AppodealIonAdsPlatform with environment variables. keepAlive: true is appropriate for a global service.
  • Verbosity: verbose: true is good for development. For production, consider making this conditional (kDebugMode ? true : false) to avoid excessive logging.
  • Security & Privacy: Ad keys are sourced from EnvVariable, avoiding hardcoding.
  • Verdict: Mostly good. Suggest making the verbose flag conditional on kDebugMode.

lib/app/services/markdown/delta_markdown_converter.dart

  • Ad Block Markdown Conversion: The text-editor-ad case in _convertEmbeddableBlock ensures ad blocks are represented as \n--ad--\n in Markdown, maintaining a clear separation.
  • Verdict: Looks good.

lib/app/services/markdown/quill.dart

  • Ad Block Quill Conversion: Consistently, text-editor-ad is added to DeltaToMarkdown's embed builders, also writing \n--ad--\n.
  • Verdict: Looks good.

lib/app/services/uuid/generate_conversation_id.dart

  • Ad Conversation ID: Including ConversationType.ad in the oneToOne case for UUID generation ensures a consistent and deterministic ID for ad "conversations", which is useful for internal tracking if ads are modeled as conversations.
  • Verdict: Looks good.

packages/ion_ads/

  • New Flutter Plugin: This entire package is new, providing a dedicated Flutter interface for Appodeal. The structure, Android setup (Kotlin files, Gradle, manifests, resources), and iOS setup (Podfile) appear standard for a Flutter plugin.
  • Native Ad Rendering: The AppodealNativeAdView and NativeAdViewBinder architecture allow for customizable native ad templates, which is a strong point for integrating ads seamlessly.
  • Privacy & Security: The inclusion of AppodealConsentManager and networkSecurityConfig indicates attention to privacy and secure network handling.
  • Logging: The apdLog function, conditionally enabled for debug builds, is helpful for debugging the native side of the plugin.
  • AdChoices Visibility: In AppodealNativeAdView.kt, adView.adChoiceView?.visibility = View.GONE is set. While the XML layouts (apd_native_ad_view_article.xml, etc.) do contain a custom ad_choices ImageView, ensure that this custom implementation is always displayed and fully compliant with ad network policies regarding ad disclosures. Hiding the default view should only be done if a compliant replacement is visible.
  • Verdict: Overall good. Actionable: Double-check AdChoices visibility and legality in AppodealNativeAdView and its associated XML layouts to ensure full compliance with ad network policies, as the default adChoiceView is hidden.

Summary of Recommendations:

  1. lib/app/features/chat/recent_chats/views/pages/recent_chats_timeline_page/recent_chats_timeline_page.dart & lib/app/features/feed/stories/providers/user_stories_provider.r.dart: For Random number generation in ad placements, consider seeding with DateTime.now().millisecondsSinceEpoch instead of list length for more varied ad experiences across app sessions.
  2. lib/app/features/feed/views/pages/feed_page/components/feed_posts_list/feed_posts_list.dart: Refine the try-catch (on Object catch (_)) block during ad insertion to either log the specific error or catch more specific exceptions for better debugging.
  3. lib/app/services/ion_ad/ion_ad_provider.r.dart: Make the verbose flag for Appodeal initialization conditional on kDebugMode to prevent excessive logging in production builds.
  4. packages/ion_ads/android/src/main/kotlin/com/appodeal/appodeal_flutter/AppodealNativeAdView.kt: Confirm that hiding the default adChoiceView (adView.adChoiceView?.visibility = View.GONE) in AppodealNativeAdView is fully compliant with ad network policies, and that the custom ad_choices implementation in the XML layouts is always visible and sufficient for legal disclosure.

Overall, this is a well-structured and comprehensive ad integration. The noted points are minor suggestions for further improvement and verification.

@github-actions

github-actions Bot commented Feb 8, 2026

Copy link
Copy Markdown

🤖 Gemini Flutter & Code Review

As a Senior Software Engineer, I've reviewed the provided code changes. This Pull Request introduces significant advertising integration, affecting platform configurations, core application logic, and UI components.

Overall, the architectural approach to integrating the ion_ads plugin and managing ad-related logic via Riverpod providers is generally sound, promoting a good separation of concerns. However, there are several critical privacy, security, and compliance concerns that require immediate attention, along with some minor performance and code style suggestions.


🛡️ General Code Quality & 📱 Flutter & Mobile Analysis

.github/workflows/gemini_workflow.yml

  • Looks good. The new workflow is correctly configured to run on PR open/sync and uses secrets appropriately.

android/app/build.gradle

  • Looks good. Correctly adds AdMob ID placeholders and Appodeal SDK dependencies.

android/app/src/main/AndroidManifest.xml

  • Security & Privacy (CRITICAL): The AD_ID permission is now added back using tools:node="replace". While ad SDKs often require this, ensure that your app's privacy policy clearly informs users about the collection and use of advertising identifiers and provides mechanisms for user consent if applicable (e.g., GDPR, CCPA).
  • Looks good. The com.google.android.gms.ads.APPLICATION_ID meta-data is correctly added using environment variables.

android/build.gradle

  • Looks good. Necessary Maven repositories for Appodeal and its adapters are added.

ios/Podfile

  • Looks good. The appodeal function is a clean way to manage the extensive list of ad network dependencies.

ios/Runner/Info.plist

  • Security & Privacy (CRITICAL): NSLocationWhenInUseUsageDescription is added with the purpose "This app needs location access for analytics and advertising purposes."
    • Action: This is a major privacy concern and potential reason for App Store rejection. You need a very strong justification for requesting location solely for ads/analytics. If location is not absolutely critical for core app functionality or a specific ad feature that users explicitly opt into, it should not be requested. Reconsider if NSLocationWhenInUseUsageDescription is truly necessary. Many ad networks can function without precise location.
  • Looks good. NSUserTrackingUsageDescription is correctly included for ATT compliance. GADApplicationIdentifier is set. The SKAdNetworkItems list is comprehensive and standard for ad network integration.

lib/app/components/text_editor/components/custom_blocks/text_editor_ad_block.dart

  • Performance Optimization: The Container using height: 286 for AppodealNativeAd imposes a fixed size. While this might be a design choice, native ads can have variable content. If the ad content doesn't fit or is much smaller, it might lead to wasted space or clipping. Consider if a more flexible sizing (e.g., IntrinsicHeight with constraints or letting the ad expand) would be beneficial, if the ad type supports it.
  • Looks good. The custom embed and builder pattern for FlutterQuill is well-implemented.

lib/app/components/text_editor/text_editor_preview.dart

  • Looks good. The ad builder is correctly added to the list of embed builders.

lib/app/features/chat/providers/conversation_messages_provider.r.dart

  • Looks good. The ad injection logic within the provider (_injectAdIfPossible) effectively separates concerns and uses appropriate constants. Using ref.watch(isCurrentUserSelectorProvider) inside a method that modifies state is fine, as long as the provider ConversationMessages is allowed to invalidate its state based on isCurrentUserSelectorProvider.

lib/app/features/chat/recent_chats/views/pages/recent_chats_timeline_page/recent_chats_timeline_page.dart

  • Architectural Consistency: The ad injection logic within useOnInit directly modifies a list that is derived from a Riverpod provider (conversations). While it creates a new list, it’s generally more idiomatic in Riverpod to have the provider itself (or a derived provider) emit the ad-interspersed list. This ensures the source of truth for the data stream is consistent and reactive.
    • Suggestion: Move the ad injection logic for conversations into the conversationsProvider or a recentChatsWithAdsProvider derived from it.
  • Code Style: Random(conversations.length) as a seed provides deterministic results given the same number of conversations. If the intent is non-deterministic ad placement across app restarts (for testing varied placements), using Random() (which uses DateTime.now().microsecondsSinceEpoch as a seed) might be more appropriate.
  • Performance Optimization: The AdChatTile uses a fixed height: 72.0.s. Similar to the text editor ad block, this might lead to sub-optimal rendering if the native ad content has different natural dimensions for the chat type.

lib/app/features/chat/views/components/message_items/message_types/text_message/text_message.dart

  • Performance Optimization: The _AdItem uses a fixed width: 246 and height: 222. This might prevent the ad from being truly responsive or adapting to varying screen sizes and ad content. Consider using flexible sizing or allowing the ad to determine its own size if possible.
  • Looks good. The conditional rendering for ads and the IONLoadingIndicatorThemed are good additions.

lib/app/features/feed/providers/parsed_media_provider.r.dart

  • Looks good. Injecting ads directly into the Quill Delta content is a clever and effective way to integrate ads into articles, maintaining the rich text structure. Random seeding is appropriate here.

lib/app/features/feed/stories/providers/user_stories_provider.r.dart

  • Looks good. The _generateAdIndexes method for story ad placement is a well-encapsulated piece of logic.

lib/app/features/feed/stories/views/components/story_viewer/components/header/story_header_gradient.dart

  • Looks good. UI aesthetic change.

lib/app/features/feed/stories/views/components/story_viewer/components/viewers/ad_story_viewer.dart

  • Looks good. Uses the existing storyImageLoadStatusProvider as a proxy for ad loading, which is pragmatic for the progress bar. AppodealNativeAd is used correctly.

lib/app/features/feed/views/pages/feed_page/components/feed_posts_list/feed_posts_list.dart

  • Error Handling: The try-on Object catch (_) block when inserting _CustomNativeAd is too broad. This can silently swallow unexpected errors, making debugging difficult.
    • Suggestion: Replace on Object catch (_) with a more specific exception type if possible, or at minimum, log the exception for monitoring.
  • Performance Optimization: _CustomNativeAd uses a fixed height: 246.0.s. Similar to other ad placements, this can limit responsiveness or lead to inconsistent UI if native ad content varies.
  • Looks good. Delegation of computeInsertionIndices to ionAdClient is a good separation of concerns.

lib/app/features/video/views/pages/ad_video_page.dart

  • Looks good. The AdVideoViewer is well-implemented for displaying video ads, including a progress bar using useAnimationController. Responsive top margin calculation is good.

lib/app/features/video/views/pages/videos_vertical_scroll_page.dart

  • Common Flutter Pitfalls (Potential Bug): The itemCount calculation for PageView.builder (flattenedVideos.length + adPositions.where((p) => p < flattenedVideos.length + 10).length) seems overly complex and potentially inaccurate.
    • Concern: If adPositions contains indices far beyond flattenedVideos.length, the itemCount might become unnecessarily large or, conversely, too small if not enough ad positions are generated relative to the actual content length.
    • Suggestion: Ensure adPositions is generated only for indices up to the current content length, and itemCount is simply flattenedVideos.length + adPositions.length. This would be more robust.
  • Code Clarity: The ad placement logic within useMemoized is quite intricate with rng.nextInt(showAdAfter) - showAdAfter / 2. While functional, adding comments to explain the desired variability and max(3, gap) logic would improve readability.

lib/app/services/ion_ad/ion_ad_provider.r.dart

  • Security & Privacy (CRITICAL): hasConsent: true is hardcoded during initialization. This is a potential compliance issue if the app operates in regions requiring explicit user consent (e.g., GDPR, CCPA).
    • Action: Ensure that user consent is obtained and reflected in this value before initialize is called. This true should ideally come from a user preference or a dedicated consent management provider.
  • Performance Optimization: verbose: true is enabled by default. While useful for development, this can generate excessive logs and potentially impact performance or increase app size in production.
    • Suggestion: Make verbose conditional on debug builds (e.g., kDebugMode).
  • Looks good. Environment variable usage for ad keys and keepAlive: true for a global service are appropriate.

lib/app/services/markdown/delta_markdown_converter.dart & lib/app/services/markdown/quill.dart

  • Looks good. Consistent mapping of text-editor-ad to and from Markdown representation.

packages/ion_ads/android/src/main/AndroidManifest.xml

  • Looks good. Adds necessary network permissions.

packages/ion_ads/android/src/main/res/xml/network_security_config.xml

  • Security (CRITICAL): android:usesCleartextTraffic="true" is enabled. This means the app will allow insecure HTTP connections. This is a security risk as data transmitted over HTTP can be intercepted.
    • Action: This should be set to false unless there's an explicit and unavoidable requirement from an ad network that cannot use HTTPS. If cleartext traffic is needed, it should be limited to specific, documented domains using <domain-config> rather than a global blanket allowance. Verify with Appodeal if they truly require this.

packages/ion_ads/android/src/main/kotlin/com/appodeal/appodeal_flutter/AppodealNativeAdView.kt

  • Privacy & Compliance (CRITICAL): adView.adChoiceView?.visibility = View.GONE hides the AdChoices icon. Ad network policies (e.g., Google AdMob, Meta Audience Network) often mandate that the AdChoices icon be clearly visible on native ads for transparency. Hiding it can lead to policy violations and ad serving limitations.
    • Action: Re-enable the visibility of the AdChoices icon or confirm with Appodeal and all mediated networks that hiding it is compliant for all ad types used.

packages/ion_ads/ios/Classes/native_ad/TemplateNativeAdViewBinderImpl.swift

  • Privacy & Compliance (CRITICAL): Similar to Android, adView.adChoiceView.isHidden = true for the AppWall native ad type.
    • Action: Re-enable the visibility of the AdChoices icon or confirm with Appodeal and all mediated networks that hiding it is compliant for all ad types used.

Summary of Critical Issues:

  1. iOS Info.plist - Location Permission Justification: NSLocationWhenInUseUsageDescription for advertising purposes is a major privacy and App Store compliance risk. Action required: Justify or remove.
  2. Android network_security_config.xml - Cleartext Traffic: usesCleartextTraffic="true" is a security vulnerability. Action required: Disable or scope to specific domains (HTTPS preferred).
  3. Native AdChoices Visibility (Android & iOS): Hiding the AdChoices icon (adChoiceView.visibility = View.GONE / isHidden = true) in native ad implementations likely violates ad network policies. Action required: Re-enable or obtain explicit compliance confirmation from Appodeal and all mediated networks.
  4. ion_ad_provider.r.dart - hasConsent: true: Hardcoding consent to true is a serious privacy compliance issue. Action required: Dynamically set this based on actual user consent collected by the app.

Recommendations & Suggestions:

  • Fixed Ad Sizes: Discuss with design/product if the fixed height/width for native ads are acceptable or if more responsive layouts are desired to better accommodate varying ad content across different devices.
  • Error Handling: Replace broad try-catch blocks with specific exception handling or comprehensive logging in feed_posts_list.dart.
  • Ad Placement Logic: Consider centralizing the ad placement strategy (intervals, types, and conditions) into a dedicated service or within the ion_ads plugin itself to make it easier to manage, test, and potentially A/B test ad density.
  • PageView.builder itemCount: Refine the itemCount calculation in videos_vertical_scroll_page.dart to be more precise and robust.
  • Verbose Logging: Make verbose: true in ion_ad_provider.r.dart conditional on debug mode.

This PR is a substantial feature, and addressing these critical items will ensure its robust and compliant operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants