Skip to content
Merged
Show file tree
Hide file tree
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 @@ -73,20 +73,44 @@ export abstract class BaseQueryBuilderService {
/* Stream that emits the initial value for some or all search filters */
populateFilters = new BehaviorSubject<{ [key: string]: any }>({});

/* Stream that emits every time queryFragments change */
queryFragmentsUpdate = new BehaviorSubject<{ [key: string]: any }>({});

/* Stream that emits every time userFacetBuckets change */
userFacetBucketsUpdate = new BehaviorSubject<{ [key: string]: FacetFieldBucket[] }>({});

categories: SearchCategory[] = [];
queryFragments: { [id: string]: string } = {};
filterQueries: FilterQuery[] = [];
filterRawParams: { [key: string]: any } = {};
paging: { maxItems?: number; skipCount?: number } = null;
sorting: SearchSortingDefinition[] = [];
sortingOptions: SearchSortingDefinition[] = [];

private encodedQuery: string;
private scope: RequestScope;
private selectedConfiguration: number;
private _userQuery = '';
private _queryFragments: { [id: string]: string } = {};

private readonly queryFragmentsHandler: ProxyHandler<{ [key: string]: any }> = {
set: (target: { [key: string]: any }, property: string, value: any) => {
target[property as keyof typeof target] = value;
this.queryFragmentsUpdate.next(this._queryFragments);
return true;
}
};

protected userFacetBuckets: { [key: string]: FacetFieldBucket[] } = {};

get queryFragments(): { [key: string]: any } {
return this._queryFragments;
}

set queryFragments(value: { [key: string]: any }) {
Copy link
Contributor

@AleksanderSklorz AleksanderSklorz Nov 27, 2025

Choose a reason for hiding this comment

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

Two things:

  1. Can we set string instead of any because I see that queryFragments field has string as value type (right side)? If it works then it would be better option
  2. If no then see following
    Any here and in setter below give eslint issue. I know you don't know what the type will be as they can have any type but can we use different type than any like "uknown" or "never"? They are a bit safer alternatives
    https://rahuulmiishra.medium.com/typescripts-any-vs-unknown-vs-never-when-and-how-to-use-them-35d81ea57c01
image

Would it have sense to use one of those types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AleksanderSklorz , minding that it's a library and type might be other than string, "unknown" would be the right choice. I've checked and all over the code inside of ADF we just once assign undefined there instead of string and that wouldn't be a problem. The problem I see is that we cannot just change it to 'unknown' because it would be a breaking change - some consumers would need type narrowing now, so I believe it should be supported by a sort of migration guide. Probably it should be a dedicated ticket for that

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me ask Michał what steps we should do in that case. @MichalKinas can you help us make decision? You can check that comment and open comments below which are related

Copy link
Contributor

Choose a reason for hiding this comment

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

Unknow may throw some TS issues when compared with any other type so I would avoid it in that case, since it can take some other types except of string I think any usage would be justified here, we can think of suppresing the eslint warning there too

this._queryFragments = this.createQueryFragmentsProxy(value);
this.queryFragmentsUpdate.next(this._queryFragments);
}

get userQuery(): string {
return this._userQuery;
}
Expand All @@ -108,6 +132,7 @@ export abstract class BaseQueryBuilderService {
protected readonly alfrescoApiService: AlfrescoApiService
) {
this.resetToDefaults();
this._queryFragments = this.createQueryFragmentsProxy({});
}

public abstract loadConfiguration(): SearchConfiguration | SearchConfiguration[];
Expand Down Expand Up @@ -146,10 +171,10 @@ export abstract class BaseQueryBuilderService {
const currentConfig = this.loadConfiguration();
if (Array.isArray(currentConfig) && currentConfig[index] !== undefined) {
this.selectedConfiguration = index;
this.configUpdated.next(currentConfig[index]);
this.searchForms.next(this.getSearchFormDetails());
this.resetSearchOptions();
this.setUpSearchConfiguration(currentConfig[index]);
this.configUpdated.next(currentConfig[index]);
this.update();
}
}
Expand Down Expand Up @@ -216,6 +241,7 @@ export abstract class BaseQueryBuilderService {
buckets.push(bucket);
}
this.userFacetBuckets[field] = buckets;
this.userFacetBucketsUpdate.next(this.userFacetBuckets);
}
}

Expand All @@ -239,6 +265,7 @@ export abstract class BaseQueryBuilderService {
if (field && bucket) {
const buckets = this.userFacetBuckets[field] || [];
this.userFacetBuckets[field] = buckets.filter((facetBucket) => facetBucket.label !== bucket.label);
this.userFacetBucketsUpdate.next(this.userFacetBuckets);
}
}

Expand Down Expand Up @@ -615,4 +642,8 @@ export abstract class BaseQueryBuilderService {
queryParamsHandling: 'merge'
});
}

private createQueryFragmentsProxy(target: { [key: string]: any }): { [key: string]: any } {
return new Proxy(target, this.queryFragmentsHandler);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { TestBed } from '@angular/core/testing';
import { ContentTestingModule } from '../../testing/content.testing.module';
import { ADF_SEARCH_CONFIGURATION } from '../search-configuration.token';
import { ActivatedRoute, Router } from '@angular/router';
import { skip } from 'rxjs/operators';

const buildConfig = (searchSettings = {}): AppConfigService => {
let config: AppConfigService;
Expand Down Expand Up @@ -840,4 +841,67 @@ describe('SearchQueryBuilder', () => {
});
});
});

describe('userFacetBucketsUpdate', () => {
it('should emit updated list of UserFacetBuckets on adding the bucket', (done) => {
const service = TestBed.inject(SearchQueryBuilderService);

service.userFacetBucketsUpdate.pipe(skip(1)).subscribe((buckets) => {
expect(buckets).toEqual({ test: [{ checked: true, filterQuery: 'f1-q1', label: 'f1-q1', count: 1 }] });
done();
});

const currentBuckets = service.getUserFacetBuckets('test');
expect(currentBuckets).toEqual([]);
service.addUserFacetBucket('test', { checked: true, filterQuery: 'f1-q1', label: 'f1-q1', count: 1 });
});

it('should emit updated list of UserFacetBuckets on removing the bucket', (done) => {
const service = TestBed.inject(SearchQueryBuilderService);
service.addUserFacetBucket('test', { checked: true, filterQuery: 'f1-q1', label: 'toStay', count: 1 });
service.addUserFacetBucket('test', { checked: true, filterQuery: 'f1-q1', label: 'toLeave', count: 1 });

service.userFacetBucketsUpdate.pipe(skip(1)).subscribe((buckets) => {
expect(buckets).toEqual({ test: [{ checked: true, filterQuery: 'f1-q1', label: 'toStay', count: 1 }] });
done();
});

const currentBuckets = service.getUserFacetBuckets('test');
expect(currentBuckets).toEqual([
{ checked: true, filterQuery: 'f1-q1', label: 'toStay', count: 1 },
{ checked: true, filterQuery: 'f1-q1', label: 'toLeave', count: 1 }
]);
service.removeUserFacetBucket('test', { checked: true, filterQuery: 'f1-q1', label: 'toLeave', count: 1 });
});
});

describe('queryFragments proxy set up', () => {
it('should emit queryFragmentsUpdate when proxy property is set', (done) => {
const service = TestBed.inject(SearchQueryBuilderService);

service.queryFragmentsUpdate.pipe(skip(1)).subscribe((fragments) => {
expect(fragments).toEqual({ test: 'test_fragment' });
done();
});

const currentFragments = service.queryFragments;
expect(currentFragments).toEqual({});
service.queryFragments['test'] = 'test_fragment';
});

it('should emit queryFragmentsUpdate when setter replaces the proxy', (done) => {
const service = TestBed.inject(SearchQueryBuilderService);

service.queryFragments['test'] = 'test_fragment';
const currentFragments = service.queryFragments;

expect(currentFragments).toEqual({ test: 'test_fragment' });

service.queryFragmentsUpdate.pipe(skip(1)).subscribe((fragments) => {
expect(fragments).toEqual({});
done();
});
service.queryFragments = {};
});
});
});
Loading