Conversation
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=lint,test,compodoc --conf... |
❌ Failed | 1m 54s | View ↗ |
nx affected --target=build,build-webcomponent,b... |
✅ Succeeded | 4m 20s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-04-29 23:20:17 UTC
🚀 Preview Deploy Report✅ Successfully deployed preview here |
|
|
||
| this.digitalObjects = toSignal(this.kg.digitalObjects(), { initialValue: {} }); | ||
| private setPageTitle() { | ||
| this.http.get('https://cdn.humanatlas.io/digital-objects/kg/digital-objects.jsonld').subscribe((data) => { |
There was a problem hiding this comment.
You will definitely have to refactor everywhere you directly reference https://cdn.humanatlas.io/digital-objects/ . This will be customized as an input to the app and can be changed based on the mirror it's deployed to.
| ontologyTree: ontologyResolver(), | ||
| cellTypeTree: cellTypeResolver(), | ||
| biomarkerTree: biomarkersResolver(), | ||
| data: (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => { |
There was a problem hiding this comment.
Refactor these functions into a helper (utils/kg-resolver.ts) instead of repeating the same logic over and over
| searchTerm: null, | ||
| }; | ||
|
|
||
| export function withFilters() { |
There was a problem hiding this comment.
This store feature should be split into 2. The first should handle the data itself, i.e. data, allRows, versionCounts. The second should be this feature and should only handle the filtering. This is the same structure we used for the cns website's current-team and research pages.
| return processedValue.charAt(0).toUpperCase() + processedValue.slice(1); | ||
| } | ||
|
|
||
| export function handleValue(value: string | string[] | undefined): string[] | undefined { |
There was a problem hiding this comment.
Rename this to something more descriptive like castArray or coerceArray
| providedIn: 'root', | ||
| }) | ||
| export class SearchService { | ||
| doSearch( |
| }, | ||
| ): Observable<string[]> { | ||
| const { organs, versions, ontologyTerms, cellTypeTerms, biomarkerTerms, searchTerm, digitalObjects } = options; | ||
|
|
There was a problem hiding this comment.
The current implementation creates a lot of intermediate arrays and iterates over the values multiple times. I would restructure something like:
search() {
const filters = [
this.createDigitalObjectFilter(digitalObjects),
this.createVersionFilter(versions)
// ...
];
const result = rows.filter((item) => filters.every(fn => fn(item)));
const ids = result.map(item => item.purl)
return of(ids);
}
private createDigitalObjectFilter(options): (item: ItemType) => boolean {
// Set#has is more efficient than Array#includes when there are a lot of options
const ids = new Set(options);
return (item) => ids.has(row.doType)
}
// Add the same for other filtersThere was a problem hiding this comment.
Use effect rather that observables
There was a problem hiding this comment.
Use effect rather than observables
There was a problem hiding this comment.
Put all this logic in a private helper method
There was a problem hiding this comment.
Put all this logic into one or more private helper methods
|
|
||
| effect(() => { | ||
| this.attachDownloadOptions(); | ||
| this.digitalObjectSearch().subscribe((results) => { |
There was a problem hiding this comment.
You should not subscribe inside of effect. Use rxResource instead
| export const environment = { | ||
| production: true, | ||
| remoteApiEndpoint: 'https://apps.humanatlas.io/api--staging', | ||
| mirrorUrl: 'https://cdn.humanatlas.io/hra-kg--staging', |
There was a problem hiding this comment.
Staging will soon have the same flat file system, so you will want to revert that once it's up (probably friday morning).
Co-authored-by: Copilot <copilot@github.com>
3 participants
Uh oh!
There was an error while loading. Please reload this page.