Skip to content
This repository has been archived by the owner on Aug 16, 2022. It is now read-only.

feat: asset filtering #81

Merged
merged 31 commits into from Mar 16, 2022
Merged

feat: asset filtering #81

merged 31 commits into from Mar 16, 2022

Conversation

KaWaite
Copy link
Member

@KaWaite KaWaite commented Nov 22, 2021

Overview

Adding infinite scroll to assets means we need to move filter/sorting and searching of assets to the backend during pagination.

What I've done

  • Updated graphql query to include sort type
  • Add sorting logic to asset querying
  • Paginated search results for asset name

Memo

  • I have made it so if the query receives before and last it'll be reversed, or first and after the results will be in normal order. Front-end will pass the appropriate params depending on if the user selects reverse order or not

@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #81 (258aad9) into main (ce23099) will decrease coverage by 0.04%.
The diff coverage is 10.38%.

❗ Current head 258aad9 differs from pull request most recent head 58df58e. Consider uploading reports for the commit 58df58e to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
- Coverage   36.32%   36.27%   -0.05%     
==========================================
  Files         315      317       +2     
  Lines       28718    28774      +56     
==========================================
+ Hits        10432    10439       +7     
- Misses      17335    17384      +49     
  Partials      951      951              
Impacted Files Coverage Δ
internal/adapter/gql/gqlmodel/convert.go 0.00% <0.00%> (ø)
internal/adapter/gql/gqlmodel/convert_asset.go 0.00% <0.00%> (ø)
internal/adapter/gql/loader_asset.go 0.00% <0.00%> (ø)
internal/adapter/gql/resolver_query.go 0.00% <0.00%> (ø)
internal/adapter/gql/resolver_team.go 0.00% <0.00%> (ø)
internal/infrastructure/memory/asset.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/mongodoc/client.go 0.00% <0.00%> (ø)
...nternal/infrastructure/mongo/mongodoc/clientcol.go 0.00% <0.00%> (ø)
...ternal/infrastructure/mongo/mongodoc/pagination.go 0.00% <0.00%> (ø)
internal/usecase/interactor/asset.go 0.00% <0.00%> (ø)
... and 9 more

@yk-eukarya yk-eukarya marked this pull request as ready for review January 24, 2022 06:04
@yk-eukarya yk-eukarya self-assigned this Jan 24, 2022
Copy link
Member

@rot1024 rot1024 left a comment

Choose a reason for hiding this comment

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

Improper design: BSON should not be used directly in use cases. The concerns about a specific DBMS (in this case MongoDB) are affecting the use case layer, which should be independent of the specific DBMS. Some model of search criteria should be defined in the domain or use case layer. That domain model is converted to BSON in each DBMS implementation, e.g. in MongoDB implementation.

internal/adapter/gql/loader_asset.go Outdated Show resolved Hide resolved
internal/usecase/repo/asset.go Outdated Show resolved Hide resolved
@yk-eukarya yk-eukarya requested a review from rot1024 March 9, 2022 08:39
internal/infrastructure/mongo/mongodoc/client.go Outdated Show resolved Hide resolved
internal/infrastructure/mongo/mongodoc/client.go Outdated Show resolved Hide resolved
internal/infrastructure/mongo/mongodoc/client.go Outdated Show resolved Hide resolved
internal/infrastructure/mongo/mongodoc/pagination.go Outdated Show resolved Hide resolved
internal/infrastructure/mongo/mongodoc/pagination.go Outdated Show resolved Hide resolved
Copy link
Member

@rot1024 rot1024 left a comment

Choose a reason for hiding this comment

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

@KaWaite found bugs:

  • SIZE and NAME sorting in reverse is messing up.
  • First query seems okay. If I ask for 20, it gives me the sorted 20.
  • But the second query, so for example ..., sort: SIZE, pagination: {last: 20, before: "EndCursor"} returns for me 19 of the original 20 sorted results.

@yk-eukarya yk-eukarya requested a review from rot1024 March 14, 2022 08:37
@yk-eukarya yk-eukarya merged commit 7399434 into main Mar 16, 2022
@yk-eukarya yk-eukarya deleted the feat/asset-filtering branch March 16, 2022 07:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants