Feature/allow kb edits#73
Conversation
jgonggrijp
left a comment
jgonggrijp
left a comment
There was a problem hiding this comment.
Looks good to me. I have some comments for your perusal.
| { | ||
| <li class="mb-3"> | ||
| @if (appMode === 'add' || appMode === 'edit') { | ||
| @if (canEditKb$ | async) { |
There was a problem hiding this comment.
Question out of curiosity: is there any performance penalty to having the same observable piped through async in two places in the same template? I'm not expecting it to be a problem here, just asking to learn for future usage.
There was a problem hiding this comment.
I had to look this up myself, and every | async call sets up a new subscription. If the observable is not shared (as we do in the case of canEditKb$ with shareReplay), I think you will get a new request to the backend for each individual subscription.
With Angular 19+, this risk of multiple backend requests can probably also be avoided by using only one async pipe in combination with @let:
@let canEdit = canEditKb$ | async;Good to keep in mind; thanks for pointing this out!
2 participants
This PR fixes a couple of issues revolving around KB items.
Creating/editing/deleting KB items is no longer limited to problem belonging to the USER dataset. All problems (incl. those from SICK, FraCaS and SNLI datasets) can now have KB annotations.
The "Add knowledge base item" and "Save problem" buttons are now visible if the user has the
annotation.changeknowledgebasepermission. Their visibility used to depend indirectly onuser.can_edit_problem, which is meant for changes to a problem's hypothesis and sentences.KB items can now be deleted. This was not possible before.