Ad configuration overhaul #38
Conversation
- Deleted AdConfig class and its generated file - Removed related tests - This model was likely unused as ad injection is now handled by the growth team's android app
- Define a new enum to specify the ad platform used across the application - Include options for Google AdMob and local custom ad platform
- Update _fe_analyzer_shared from 85.0.0 to 88.0.0 - Update analyzer from 7.7.1 to 8.1.1 - Update dart_style from 3.1.1 to 3.1.2 - Update json_serializable from 6.10.0 to 6.11.0 - Update watcher from 1.1.2 to 1.1.3
Defines specific, standard locations for ads within an article's content, including positions below the main image, above/below the continue reading button.
- Create LocalAd class to define a single custom ad - Include properties for id, title, subtitle, imageUrl, targetUrl, and adType - Implement JSON serialization and deserialization - Add copyWith method for easy instance modification - Use Equatable for equality comparisons
- Import `meta` package for additional meta-information - Improve code documentation and type checking
- Create new class to hold ad identifiers for specific platforms - Implement Equatable for value comparison - Add JSON serialization support with json_annotation - Include properties for various ad types: feed native, feed banner, article interstitial, in-article native, and in-article banner - Provide copyWith method for easy instance copying with modifications
- Create new model for user-role-based ad frequency and placement intervals - Implement Equatable for value comparison - Add JSON serialization support with json_annotation - Include fields for guest, authenticated, and premium user ad frequencies and placement intervals - Provide copyWith method for easy instance modification
- Create ArticleInterstitialAdConfiguration class - Add properties for enabled, adType, and frequencyConfig - Implement JSON serialization and deserialization - Include copyWith method for easy instance manipulation
- Create new model for master configuration of all ads on article pages - Include properties for enabled status, default in-article ad type, interstitial ad configuration, and in-article ad slot configurations - Add methods for JSON serialization and copying with updated values
- New model for user-role-based frequency of interstitial ads on article pages - Includes properties for guest, standard user, and premium user article read counts before ad display - Implements Equatable for value comparison and JsonSerializable for JSON conversion
- Create new model for ads in feed-like sections - Include master switch for enabling/disabling feed ads - Define ad type (native or banner) for entire feed placement - Add user-role-based frequency and placement intervals
- Create new model for configuring individual in-article ad slots - Include properties for slot type and enabled status - Implement JSON serialization and deserialization - Add copyWith method for easy modification
- Deleted the extra blank line at the end of the file to improve code formatting consistency.
- Create AdConfig class to serve as the main container for all ad-related configurations - Include properties for primary ad platform, platform identifiers, local ads catalog, feed ad configuration, and article ad configuration - Implement JSON serialization and deserialization - Add copyWith method for easy modification of individual properties
- Add new ad platform types and in-article ad slot types enums - Introduce article and feed ad configuration models - Include app status and user preference config models
…ings - Replace simple ad frequency settings with comprehensive AdConfig model - Add local ads catalog and platform-specific ad identifiers - Define feed and article ad configurations with frequency settings - Include in-article ad slot configurations
- Verify string values of AdPlatformType - Test creation of AdPlatformType from string values - Check toString representation of AdPlatformType
- Add unit tests for InArticleAdSlotType enum - Verify string values, creation from string, and toString representation
- Add unit tests for LocalAd class - Cover instantiation, value equality, copyWith, and JSON serialization
- Instantiate test objects for AdMob and Local platforms - Verify correct property values for AdMob and Local fixtures - Test value equality and copyWith functionality - Implement fromJson/toJson round trip tests for both platforms
- Add unit tests for ArticleInterstitialAdConfiguration model - Cover instantiation, value equality, copyWith, and JSON serialization - Include test case for invalid adType
- Add unit tests for ArticleInterstitialAdFrequencyConfig model - Verify instantiation, value equality, copyWith functionality, and JSON serialization/deserialization
- Add unit tests for FeedAdConfiguration class - Cover instantiation, value equality, copyWith method, and JSON serialization - Include test case for adType validation
- Add unit tests for FeedAdFrequencyConfig model - Test instantiation, value equality, copyWith method, and JSON serialization
- Add unit tests for InArticleAdSlotConfiguration class - Cover instantiation, value equality, copyWith method, and JSON serialization - Ensure proper functionality of the model class
- Add unit tests for ArticleAdConfiguration class - Test instantiation, value equality, copyWith method, and JSON serialization - Verify defaultInArticleAdType validation
- Instantiate test for AdConfig - Equality test for AdConfig - CopyWith test for AdConfig - Json round trip test for AdConfig
- Add adConfig to the list of properties copied in the RemoteConfig copyWith method - Update the remote config test to include adConfig comparison - Add a new test case to verify the primaryAdPlatform can be updated correctly
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @fulleni, 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 significant overhaul of the application's ad configuration system. The primary goal is to enhance flexibility and control over ad placements and behaviors by moving from a flat, less descriptive configuration to a highly structured and modular approach. This change allows for better management of different ad platforms, granular control over ad frequencies and placements in various sections like feeds and articles, and the ability to define local ad content. The refactoring ensures that the ad system is more maintainable, scalable, and adaptable to future advertising requirements.
Highlights
- AdConfig Model Overhaul: The core
AdConfigmodel has been completely restructured to support a more flexible and extensible ad system. Instead of flat properties, it now uses nested configuration objects for different ad types and placements. - New Ad-Related Enums: Introduced new enums (
AdPlatformType,InArticleAdSlotType) to clearly define ad platforms (e.g., AdMob, local) and specific in-article ad placement locations, enhancing clarity and control. - Granular Ad Configuration Models: New dedicated configuration models (
AdPlatformIdentifiers,FeedAdConfiguration,ArticleAdConfiguration,LocalAd, etc.) have been added to manage ad unit IDs, feed ad settings, article ad settings (including interstitial and in-article slots), and local ad content with greater granularity. - Fixture Updates: The
remote_configs.dartfixture has been updated to demonstrate and utilize the new structured ad configuration, providing a clear example of how the new system will be implemented. - Expanded Test Coverage: Comprehensive unit tests have been added or updated for all new and modified ad configuration models and enums, ensuring the reliability and correctness of the new system.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed overhaul of the ad configuration system. The new structure is much more organized, scalable, and maintainable by breaking down the monolithic AdConfig into smaller, more focused models. The addition of comprehensive tests for each new model is excellent. I have a couple of minor suggestions to further improve code clarity and documentation.
- Add detailed explanation of AdFrequency and AdPlacementInterval - Illustrate how these two parameters work together to control ad injection - Remove unnecessary import of 'meta/meta.dart'
Status
READY
Description
This pull request introduces a significant overhaul of the application's ad configuration system. The primary goal is to enhance flexibility and control over ad placements and behaviors by moving from a flat, less descriptive configuration to a highly structured and modular approach. This change allows for better management of different ad platforms, granular control over ad frequencies and placements in various sections like feeds and articles, and the ability to define local ad content. The refactoring ensures that the ad system is more maintainable, scalable, and adaptable to future advertising requirements.
Type of Change