Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editor: add context for remote typeahead #403

Merged
merged 1 commit into from
Jun 1, 2021
Merged

editor: add context for remote typeahead #403

merged 1 commit into from
Jun 1, 2021

Conversation

sebdeleze
Copy link

Adds the currently edited record PID to suggestions backend request, to be able to do a specific process with the given PID.

Co-Authored-by: Sébastien Délèze sebastien.deleze@rero.ch

Adds the currently edited record PID to suggestions backend request, to be able to do a specific process with the given PID.

Co-Authored-by: Sébastien Délèze <sebastien.deleze@rero.ch>
@sebdeleze sebdeleze marked this pull request as ready for review May 31, 2021 14:48
@sebdeleze sebdeleze requested review from jma, Garfield-fr and zannkukai and removed request for jma May 31, 2021 14:48
constructor(
private _remoteTypeaheadService: RemoteTypeaheadService
) {
constructor(private _remoteTypeaheadService: RemoteTypeaheadService, private _route: ActivatedRoute) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems better to have each constructor argument on separate line.

Copy link
Author

Choose a reason for hiding this comment

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

Prettier says no...

@@ -102,12 +104,18 @@ export class RemoteTypeaheadService {
})
);
} else {
const filters: any = {};
if (currentPid) {
filters.currentPid = currentPid;
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't well understand how this filter can impact the search result.
If I'm right, using this syntax the url sended to backend will be something like :

http://endpoint/api/resource?q=[...]&currentPid=[currentPid]

So the currentPid url argument must be manage by the query handler into backend ? (it seems not the case for RERO-ILS, maybe OK for sonar ?)

Copy link
Author

Choose a reason for hiding this comment

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

You're right, it has to be managed in backend query. In most cases it's OK without that, but I have a collections resource in SONAR that can be hierarchical. A collection can have a parent collection. And I want to avoid to suggest the same collection as the current one.

@sebdeleze sebdeleze merged commit e1bcc11 into rero:dev Jun 1, 2021
@sebdeleze sebdeleze deleted the sed-remotetypehead-current-record branch June 1, 2021 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants