Add find_by_filters method to support dynamic querying, pagination, sorting, and field selection

[ddeerrrriicckk]

Introduce query capabilities to the Recommendations model to support dynamic filtering based on various attributes, including single conditions, multiple conditions, pagination, sorting, date range filtering, and field selection.

Changes:

1. Added find_by_filters Method:

   • Supports querying recommendations based on filters including product_id, recommended_id, recommendation_type, status, created_at_min, and created_at_max.
   • Implements pagination using page and limit parameters.
   • Supports sorting with sort_by and order parameters.
   • Enables field selection through the fields parameter to specify which fields to return.

2. Added helper methods:

   • _apply_filters, _apply_date_range, _apply_sorting, and _apply_pagination manage filtering, date range, sorting, and pagination logic separately.
   • _select_fields method ensures only specified fields are included in the query results.

Test Coverage:

• Single-condition and multiple-condition filtering
• Pagination
• Sorting, ensuring results are correctly ordered by created_at
• Date range filtering, with boundary testing for start and end dates
• Field selection, ensuring only specified fields are returned
• Non-matching conditions, verifying an empty result list is returned
• Invalid parameter handling to ensure robustness

[muyunli123]

please correct me if I'm wrong: I noticed that you added a new method in models.py which includes the functionalities of find by all the attributes. Currently we have a find_by_product_id() and find_by_recommended_id() methods in models.py, which I think serve part of the functions of querying. Since your new method already include those functions, could you delete the 2 existing method and the corresponding test cases as well? Also, I see that in our LIST route, we have if-statements to check if certain attributes are included in the information passed into it and call find_by_product_id() and find_by_recommended_id() when needed, could you add more if-statements according to the new find_by_filters() method (and test cases in test_routes.py if necessary)? Thank you!

[ddeerrrriicckk]

please correct me if I'm wrong: I noticed that you added a new method in models.py which includes the functionalities of find by all the attributes. Currently we have a find_by_product_id() and find_by_recommended_id() methods in models.py, which I think serve part of the functions of querying. Since your new method already include those functions, could you delete the 2 existing method and the corresponding test cases as well? Also, I see that in our LIST route, we have if-statements to check if certain attributes are included in the information passed into it and call find_by_product_id() and find_by_recommended_id() when needed, could you add more if-statements according to the new find_by_filters() method (and test cases in test_routes.py if necessary)? Thank you!

This PR is focused on the model story (Add Query Capability to Database Model). The newly added find_by_filters method indeed includes the functionality of previous methods like find_by_product_id and find_by_recommended_id. However, I chose not to delete these existing methods because they are already used in the current routing logic and are part of a previous story. Removing them directly would lead to changes in existing functionality.

Specifically:

Maintaining Code Consistency and Stability: The find_by_product_id and find_by_recommended_id methods are closely tied to the current LIST route functionality and may involve existing test coverage. Therefore, within the scope of this story, I retained these methods to avoid unnecessary impacts on the current routes and functionality.

Step-by-Step Migration: My intention is for find_by_filters to serve as a more general query method, which could eventually (in “Add Query capability to LIST method by product id” story or other future route-focused story) replace single-attribute query methods like find_by_product_id and find_by_recommended_id. We can progressively optimize the code in future stories, moving the route logic over to find_by_filters, and thus remove duplicate methods gradually.

Keeping Story Independence: The focus of this current story is to add querying capability at the model layer, not to modify or refactor existing routing functionality. Therefore, I suggest handling modifications to the current if-else query logic in a future story. This approach will help to reduce the complexity of the current task and keep the code more stable.

[muyunli123]

please correct me if I'm wrong: I noticed that you added a new method in models.py which includes the functionalities of find by all the attributes. Currently we have a find_by_product_id() and find_by_recommended_id() methods in models.py, which I think serve part of the functions of querying. Since your new method already include those functions, could you delete the 2 existing method and the corresponding test cases as well? Also, I see that in our LIST route, we have if-statements to check if certain attributes are included in the information passed into it and call find_by_product_id() and find_by_recommended_id() when needed, could you add more if-statements according to the new find_by_filters() method (and test cases in test_routes.py if necessary)? Thank you!

This PR is focused on the model story (Add Query Capability to Database Model). The newly added find_by_filters method indeed includes the functionality of previous methods like find_by_product_id and find_by_recommended_id. However, I chose not to delete these existing methods because they are already used in the current routing logic and are part of a previous story. Removing them directly would lead to changes in existing functionality.

Specifically:

Maintaining Code Consistency and Stability: The find_by_product_id and find_by_recommended_id methods are closely tied to the current LIST route functionality and may involve existing test coverage. Therefore, within the scope of this story, I retained these methods to avoid unnecessary impacts on the current routes and functionality.

Step-by-Step Migration: My intention is for find_by_filters to serve as a more general query method, which could eventually (in “Add Query capability to LIST method by product id” story or other future route-focused story) replace single-attribute query methods like find_by_product_id and find_by_recommended_id. We can progressively optimize the code in future stories, moving the route logic over to find_by_filters, and thus remove duplicate methods gradually.

Keeping Story Independence: The focus of this current story is to add querying capability at the model layer, not to modify or refactor existing routing functionality. Therefore, I suggest handling modifications to the current if-else query logic in a future story. This approach will help to reduce the complexity of the current task and keep the code more stable.

I see. Then I guess the current code already had the functionality of querying by attributes (product_id and recommended_id), and the enhancement you added can be optimized to replace them. I first thought that the 2 stories Add Query capability to database model #32 and Add Query capability to LIST method by product id #24 are somehow duplicated. But since we already have the capability of querying by attributes, what do you think we should deal with the Add Query capability to database model #32? Should we continue to work on it in this sprint or we move it to icebox and pick it up some time in the future?

[ddeerrrriicckk]

please correct me if I'm wrong: I noticed that you added a new method in models.py which includes the functionalities of find by all the attributes. Currently we have a find_by_product_id() and find_by_recommended_id() methods in models.py, which I think serve part of the functions of querying. Since your new method already include those functions, could you delete the 2 existing method and the corresponding test cases as well? Also, I see that in our LIST route, we have if-statements to check if certain attributes are included in the information passed into it and call find_by_product_id() and find_by_recommended_id() when needed, could you add more if-statements according to the new find_by_filters() method (and test cases in test_routes.py if necessary)? Thank you!

This PR is focused on the model story (Add Query Capability to Database Model). The newly added find_by_filters method indeed includes the functionality of previous methods like find_by_product_id and find_by_recommended_id. However, I chose not to delete these existing methods because they are already used in the current routing logic and are part of a previous story. Removing them directly would lead to changes in existing functionality. Specifically: Maintaining Code Consistency and Stability: The find_by_product_id and find_by_recommended_id methods are closely tied to the current LIST route functionality and may involve existing test coverage. Therefore, within the scope of this story, I retained these methods to avoid unnecessary impacts on the current routes and functionality. Step-by-Step Migration: My intention is for find_by_filters to serve as a more general query method, which could eventually (in “Add Query capability to LIST method by product id” story or other future route-focused story) replace single-attribute query methods like find_by_product_id and find_by_recommended_id. We can progressively optimize the code in future stories, moving the route logic over to find_by_filters, and thus remove duplicate methods gradually. Keeping Story Independence: The focus of this current story is to add querying capability at the model layer, not to modify or refactor existing routing functionality. Therefore, I suggest handling modifications to the current if-else query logic in a future story. This approach will help to reduce the complexity of the current task and keep the code more stable.

I see. Then I guess the current code already had the functionality of querying by attributes (product_id and recommended_id), and the enhancement you added can be optimized to replace them. I first thought that the 2 stories Add Query capability to database model #32 and Add Query capability to LIST method by product id #24 are somehow duplicated. But since we already have the capability of querying by attributes, what do you think we should deal with the Add Query capability to database model #32? Should we continue to work on it in this sprint or we move it to icebox and pick it up some time in the future?

I have implemented the necessary functionality as specified in story #32, which focuses on adding query capabilities to the database model. The requirements outlined in #32 were centered around enhancing the model layer with a flexible filtering method (find_by_filters) to support future needs.

Any additional changes to the existing routes or adjustments to the LIST method would fall under separate requirements.

[muyunli123]

please correct me if I'm wrong: I noticed that you added a new method in models.py which includes the functionalities of find by all the attributes. Currently we have a find_by_product_id() and find_by_recommended_id() methods in models.py, which I think serve part of the functions of querying. Since your new method already include those functions, could you delete the 2 existing method and the corresponding test cases as well? Also, I see that in our LIST route, we have if-statements to check if certain attributes are included in the information passed into it and call find_by_product_id() and find_by_recommended_id() when needed, could you add more if-statements according to the new find_by_filters() method (and test cases in test_routes.py if necessary)? Thank you!

This PR is focused on the model story (Add Query Capability to Database Model). The newly added find_by_filters method indeed includes the functionality of previous methods like find_by_product_id and find_by_recommended_id. However, I chose not to delete these existing methods because they are already used in the current routing logic and are part of a previous story. Removing them directly would lead to changes in existing functionality. Specifically: Maintaining Code Consistency and Stability: The find_by_product_id and find_by_recommended_id methods are closely tied to the current LIST route functionality and may involve existing test coverage. Therefore, within the scope of this story, I retained these methods to avoid unnecessary impacts on the current routes and functionality. Step-by-Step Migration: My intention is for find_by_filters to serve as a more general query method, which could eventually (in “Add Query capability to LIST method by product id” story or other future route-focused story) replace single-attribute query methods like find_by_product_id and find_by_recommended_id. We can progressively optimize the code in future stories, moving the route logic over to find_by_filters, and thus remove duplicate methods gradually. Keeping Story Independence: The focus of this current story is to add querying capability at the model layer, not to modify or refactor existing routing functionality. Therefore, I suggest handling modifications to the current if-else query logic in a future story. This approach will help to reduce the complexity of the current task and keep the code more stable.

I see. Then I guess the current code already had the functionality of querying by attributes (product_id and recommended_id), and the enhancement you added can be optimized to replace them. I first thought that the 2 stories Add Query capability to database model #32 and Add Query capability to LIST method by product id #24 are somehow duplicated. But since we already have the capability of querying by attributes, what do you think we should deal with the Add Query capability to database model #32? Should we continue to work on it in this sprint or we move it to icebox and pick it up some time in the future?

I have implemented the necessary functionality as specified in story #32, which focuses on adding query capabilities to the database model. The requirements outlined in #32 were centered around enhancing the model layer with a flexible filtering method (find_by_filters) to support future needs.

Any additional changes to the existing routes or adjustments to the LIST method would fall under separate requirements.

Sorry, I copied the wrong story. I meant to ask you about Add Query capability to LIST method by product id #24.

[agustdmybae]

Story #24 should be working on the LIST route. The route part isn't complete yet, just like you mentioned we need to add more if-statements using the find_by_filters() method. So we don't have to delete #24 this story!

Back to this story, the implementation for database model looks good for me. I also agree not deleting the original functions, and we'll see how we can optimize in future sprints.

[muyunli123]

Story #24 should be working on the LIST route. The route part isn't complete yet, just like you mentioned we need to add more if-statements using the find_by_filters() method. So we don't have to delete #24 this story!

Back to this story, the implementation for database model looks good for me. I also agree not deleting the original functions, and we'll see how we can optimize in future sprints.

Got it. Then we should be good to merge the PR.