Skip to content
This repository has been archived by the owner on Apr 12, 2021. It is now read-only.

feat(projects): #1594 filtered the public projects having > 1 repo #1598

Open
wants to merge 5 commits into
base: v0.11.9
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Contributor

@eddiejaoude eddiejaoude Nov 5, 2019

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)

Copy link
Contributor

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.

Copy link
Contributor Author

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 ?

Copy link
Contributor

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

Copy link
Contributor Author

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

} else {
this.showTitle = true;
this.projectSubscription = this.projectService
Expand Down