Skip to content

Add next features#2

Open
morganchen12 wants to merge 7 commits intomainfrom
mc/next
Open

Add next features#2
morganchen12 wants to merge 7 commits intomainfrom
mc/next

Conversation

@morganchen12
Copy link
Copy Markdown
Collaborator

No description provided.

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 transitions the project to a local Firebase iOS SDK dependency and updates imports from FirebaseAI to FirebaseAILogic. It significantly refines the recipe filtering UI by introducing a Form-based layout, adding a search field for instructions, and improving the rating and tag selection components. Additionally, the data model and store were updated to dynamically calculate likes and average ratings using Firestore pipelines. Feedback includes ensuring the base pipeline consistently includes aggregated fields to prevent UI data loss, removing debug print statements, and renaming a custom error type to avoid shadowing standard library enums.

Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift
Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift Outdated
Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift Outdated
Comment thread FriendlyMeals/FriendlyMeals/Shared/Models/Recipe.swift Outdated
Comment thread FriendlyMeals/FriendlyMeals/Shared/Models/Recipe.swift Outdated
@morganchen12
Copy link
Copy Markdown
Collaborator Author

/gemini review

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 transitions the project to a local Firebase SDK reference, renames the FirebaseAI module to FirebaseAILogic, and enhances the recipe filtering system. Key changes include adding a search field for recipe instructions, displaying like counts, and refactoring the Recipe model to compute average ratings dynamically using Firestore pipelines. Review feedback identifies a critical issue where the local SDK path will break external builds and points out several flaws in the Firestore pipeline logic, such as the requirement for the search stage to be positioned first and the need to ensure the averageRating field is present when sorting by rating to avoid query failures.

Comment thread FriendlyMeals/FriendlyMeals.xcodeproj/project.pbxproj Outdated
Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift
Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift
Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift
Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift Outdated
@morganchen12
Copy link
Copy Markdown
Collaborator Author

/gemini review

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 updates the project dependencies, notably replacing FirebaseAI with FirebaseAILogic and adding FoundationModels.framework. The UI for the FilterView has been significantly overhauled using a Form-based layout within a NavigationStack, and a new search filter for recipe instructions has been introduced. Additionally, the app now supports displaying and sorting by 'likes' using Firestore pipelines for real-time aggregation. Review feedback suggests refactoring duplicated pipeline logic in RecipeStore, optimizing pipeline stages by combining addFields calls, and restoring the averageRating property to the Recipe model to prevent data loss in the UI when sorting or filtering by rating.

Comment thread FriendlyMeals/FriendlyMeals/Shared/Services/RecipeStore.swift
Comment on lines +93 to +109
filters = filters.define([Field("__name__").documentId().as("parentRecipeId")]).addFields([
db.pipeline()
.collection("likes")
.where(Field("recipeId").equal(Variable("parentRecipeId")))
.aggregate([CountAll().as("count")])
.toScalarExpression()
.as("likes")
])
// Add rating sometimes
if shouldAddAverageRating {
filters = filters.addFields([
Subcollection(RecipeStore.reviewsSubcollection)
.aggregate([Field("rating").average().as("averageRating")])
.toScalarExpression()
.as("averageRating")
])
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Multiple addFields stages are being added sequentially. These can be combined into a single addFields call with an array of augmentations to reduce the number of stages in the pipeline and potentially improve execution efficiency.

    // Always add likes
    filters = filters.define([Field("__name__").documentId().as("parentRecipeId")])

    var augmentations: [Augmentation] = [
      db.pipeline()
        .collection("likes")
        .where(Field("recipeId").equal(Variable("parentRecipeId")))
        .aggregate([CountAll().as("count")])
        .toScalarExpression()
        .as("likes")
    ]

    // Add rating sometimes
    if shouldAddAverageRating {
      augmentations.append(
        Subcollection(RecipeStore.reviewsSubcollection)
          .aggregate([Field("rating").average().as("averageRating")])
          .toScalarExpression()
          .as("averageRating")
      )
    }

    filters = filters.addFields(augmentations)

Comment thread FriendlyMeals/FriendlyMeals/Shared/Models/Recipe.swift
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