-
Notifications
You must be signed in to change notification settings - Fork 0
feat- enforcing-user-content-limits #139
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
feat- enforcing-user-content-limits #139
Conversation
- Define LimitType enum for different content limits - Create LimitReachedArguments class for passing data to LimitReachedPage - Include limitType and userRole in LimitReachedArguments
- Implement UserLimitService to handle user content limits based on role and preferences - Add methods to check limits for followed topics, sources, countries, and saved headlines - Include helper methods to retrieve specific limits based on user role
- Implement `followedItemsLimit` getter to return max followed items based on user role - Implement `savedHeadlinesLimit` getter to return max saved headlines based on user role - Both getters return 0 if remote config or user info is unavailable
- Add import for UserLimitService - Initialize UserLimitService instance in bootstrap function - Pass UserLimitService to NewsApp constructor
- Import UserLimitService in app_bloc.dart - Add UserLimitService parameter to AppBloc constructor - Store UserLimitService instance as a private field in AppBloc
- Import UserLimitService in app.dart - Add UserLimitService to App class constructor and properties - Provide UserLimitService in MultiRepositoryProvider - Pass UserLimitService to AppView widget
- Implement follow limits for topics, sources, and countries - Add navigation to LimitReachedPage when limits are exceeded - Refactor following logic to check limits before adding new items - Integrate UserLimitService for limit checks - Update EntityDetailsBloc to handle navigation and limit configurations
- Add limit check before adding a headline to saved headlines - Display different messages/actions based on user role (premium/guest/standard) - Integrate UserLimitService and access remote config for limit settings - Handle null safety improvements - Add navigation to LimitReachedPage for certain user roles when limit is reached
- Add LimitReachedPage and corresponding route - Include UserLimitService in router dependencies - Update routes.dart with new limit reached route constants
- Add Arabic and English strings for followed items and saved headlines limits - Include messages for guest users, standard users, and premium users - Introduce generic limit reached messages - Add placeholder for upgrade to premium functionality
- Add LimitReachedPage widget to handle content limits for different user roles - Implement logic to display appropriate messaging and actions based on limit type and user role - Include navigation to authentication and upgrade pages (to be implemented) - Add generic limit reached message for unspecified conditions
/gemini summary |
Summary of ChangesThis pull request implements a robust system for enforcing user content limits across various features, such as following content and saving headlines. It introduces a dedicated service to determine limits based on user roles and remote configurations, and provides a user-friendly, localized interface to inform users when a limit is reached, guiding them towards appropriate actions like signing in or upgrading their subscription. Highlights
Changelog
Activity
|
/gemini review |
It looks like you're trying to invoke a command, but you've already received a summary and review. If you have further questions or would like a more detailed review of specific aspects, please let me know! |
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 robust system for enforcing user content limits, a new UserLimitService
, and a corresponding LimitReachedPage
. The implementation is solid, but there are a few areas for improvement regarding code duplication, robustness, and a potential critical bug in the limit checking logic. My review includes suggestions to centralize business logic, handle null values more safely, and correct the limit enforcement calculation. Addressing these points will enhance the maintainability and correctness of this new feature.
- Extract icon logic to a separate method - Simplify action button creation - Reduce code duplication for different limit types - Use a single action button instead of multiple actions - Refactor switch statement to if-else conditions
- Change navigation route from authentication to linking request code - This ensures users are directed to the correct page when the action button is pressed
- Update Arabic and English strings to remove the "upgrade" option - Simplify the message to focus on sign-in for following more items
- Change background to transparent with dialog - Improve layout and spacing - Enhance close button functionality - Adjust action button spacing
- Add logic to check for redirectPath in GoRouterState after successful authentication - Navigate to the redirectPath if present, replacing the current route stack - This allows users to be redirected to their previous location after signing in
…t and redirect features - Add authContext and redirectPath parameters for greater flexibility - Implement conditional close button based on authContext and redirectPath - Update navigation logic to respect redirectPath - Append query parameters for authentication flow context
…on context - Add 'authContext' and 'redirectPath' parameters to EmailCodeVerificationPage - These new parameters allow for more flexible navigation post-authentication - Prepare for integration with updated/limit_reached screen and other auth flows
- Add authContext and redirectPath parameters to RequestCodePage - Implement conditional navigation based on authContext and redirectPath - Preserve authContext across different routes in the authentication flow - Refactor back navigation logic to support flexible routing - Update VerifyCodePage navigation to handle authContext and redirectPath
- Include current URI as redirectPath when followedTopics, followedSources, or followedCountries limits are reached - Use _appBloc.navigatorKey.currentContext?.routerDelegate.currentConfiguration.uri.toString() to get current URI - Update affected areas in handleFollowTopic, handleFollowSource, and handleFollowCountry methods
- Capture current route path using GoRouter - Pass captured path as redirectPath argument to LimitReachedArguments - This allows redirection back to the original page after limit is resolved
- Add nullable redirectPath field to specify where to redirect after addressing the limit - Update props to include redirectPath
- Implement modal dialog design for limit reached page - Add navigation logic for closing modal or performing authentication - Update UI components and layout to match new design - Enhance accessibility with proper tooltips and navigation
…imit reached - Add new translations for Arabic and English - Introduce headline and subheadline for the authentication limit reached button - Ensure proper formatting and context descriptions for new translations
- Add new localization strings for authentication limit reached headline and subheadline in AppLocalizations - Implement translations for these new strings in both English and Arabic localization files
- Add support for 'limit reached' authentication context - Implement redirect path functionality in authentication flow - Refactor query parameter handling in auth routes - Update UI text and behavior based on new authentication contexts - Improve logging and debugging information in router
…imit reached logic - Replace direct access to routerDelegate with GoRouter.of() for better null safety - Add null checks before accessing routerDelegate to prevent potential crashes - Add comments to clarify the early return pattern for limit reached scenarios
- Change query parameter from 'context' to 'authContext' to avoid potential conflicts with other query parameters
…er redirects - Add helper function `_buildRedirectUri` to preserve `redirectPath` during redirects - Update redirect logic in various cases to use `_buildRedirectUri` - Ensure consistent behavior when `redirectPath` is present in query parameters
…ontext issues - Remove direct navigation logic from AppBloc after successful authentication - Update logging to reflect new navigation handling by GoRouter - Add log message for user logout - Improve code maintainability and avoid potential issues with BuildContext usage
- Implement logic to redirect users to their intended page after completing an authentication flow - Ensure redirection occurs for both authenticated and anonymous users - Add checks to prevent unnecessary redirects when the user is already on the intended page - Include debug print statement for tracking redirect events
- The redirect logic in lib/router/router.dart has been refactored. The conditional statements were reordered to ensure that a redirectPath, if present, is prioritized for navigation after a user authenticates. This corrects the bug where users were always sent to the feed. I have also added extensive logging to the redirect function to trace its behavior for easier maintenance.
Status
CANCELED
Description
This pull request implements a robust system for enforcing user content limits across various features, such as following content and saving headlines. It introduces a dedicated service to determine limits based on user roles and remote configurations, and provides a user-friendly, localized interface to inform users when a limit is reached, guiding them towards appropriate actions like signing in or upgrading their subscription.
closes #138
Type of Change