-
-
Notifications
You must be signed in to change notification settings - Fork 42
feat(projects): #1594 filtered the public projects having > 1 repo #1598
base: v0.11.9
Are you sure you want to change the base?
Conversation
I have tagged the wrong issue in the commit 7ce1111 Please squash commit while merge. if possible . It would be really helpful |
web/src/app/shared/components/private-public-project/private-public-project.component.ts
Outdated
Show resolved
Hide resolved
@@ -40,7 +40,8 @@ export class PrivatePublicProjectComponent implements OnInit, OnDestroy { | |||
if (this.router.url === '/') { | |||
this.projectSubscription = this.projectService | |||
.findPublicProjects() | |||
.subscribe((projects: IProject[]) => this.projects = projects.map((project: IProject) => new ProjectModel(project))); | |||
.subscribe((projects: IProject[]) => this.projects = projects.map((project: IProject) => new ProjectModel(project)) | |||
.filter((project: ProjectModel) => project.repositories.length > 0)); |
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.
When the list get longer in the future it would be more efficient to get the database to do the filtering if possible.
Currently the query limits to 10 results, if we remove items after the query we could end up with less than 10
ref.where('type', '==', 'public')
.orderBy('updatedOn', 'desc').limit(10)
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.
It does not look like it is possible to do a repositories.length > 0
query in Firestore. We would need to store the repositories
count to do this.
Best to store the repository length, so we can do a query to Firestore.
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.
Okay Do I have to make this change in this issue ? or we can do it in another ?
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.
I think do it in this PR, as this PR is tiny
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.
okay I will do it here
closes #1594
Notes
A summary of what was achieved in this PR