Feature/user problems #53
Conversation
...end/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html
Show resolved
Hide resolved
|
Alright, I think I've got everything! Let me know if you think this is ready :) |
jgonggrijp
left a comment
jgonggrijp
left a comment
There was a problem hiding this comment.
I have not read all parts of the diff with equal attention, because it is a large diff. I also do not really have an opinion on your overall approach, because I am still very new to the project and also because I am a bit rusty on Angular. That being said, I could recognize what you've done as basically adjusting and extending existing code to accomodate a new problem type. As such, the changes look reasonable to me. I also like that you added a substantial amount of nontrivial unittests.
Apart from that, I noticed that you don't seem very familiar yet with DRF and with pytest. You might like to try the long-form DRF tutorial and pytest's quickstart and how-to guides (I especially recommend the one on fixtures).
|
|
||
| ngOnInit(): void { | ||
| const editParam$ = this.route.url.pipe( | ||
| map(segments => segments.some(segment => segment.path === "edit")), |
There was a problem hiding this comment.
I don't actually recommend that you do this because the project is not yet using Underscore, but I always like to point out nice ways to enable shorter notation like this:
| map(segments => segments.some(segment => segment.path === "edit")), | |
| map(segments => _.some(segments, {path: "edit"})), |
(You could even do map(_.partial(_.some, _, {path: 'edit'})), though that is admittedly less recognizable for most people.)
There was a problem hiding this comment.
Interesting. Would the type checker know about path has to be a property of an element of segments? Would it complain if I accidentally wrote ptah (or some other Egyptian god)?
There was a problem hiding this comment.
The people who wrote @types/underscore (and /lodash) went to great lengths to enable TS to infer such things. In this case, I think it might be able to catch the typo. That being said, JS is at its core a dynamically typed language and some of its types are just untraceable without running the program. Underscore has not restricted itself to the subset of JS that TS can accurately type. For example, I know that _.flatten has a type that TS cannot express because of the nested infinities involved.
This PR enables the user to add their own problems and save them to the DB. Problems added by the user can be edited by going into a special 'Edit' mode. Edits are now made to the Problem objects directly; in the future we might use annotations instead.
KB items are now also editable/viewable. They were not properly queried before.