[Refactor] Notes#11971
Conversation
SchrodingersGat
added
api
✅ Deploy Preview for inventree-web-pui-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@matmair @wolflu05 before I go too far with this I'd be interested in your feedback around two major points: Data Structure Refactor: Are we on board with the idea of supporting multiple notes per item? Editor: Do you have any objections to moving to a new editor, and move to HTML notes? We will obviously have to tackle any data sanitizing required here |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #11971 +/- ##
==========================================
- Coverage 91.44% 90.94% -0.50%
==========================================
Files 976 984 +8
Lines 52162 52596 +434
==========================================
+ Hits 47701 47835 +134
- Misses 4461 4761 +300
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
I think the general idea is very good; a few points/ideas:
I like blocknote as a choice, the OpenProject and a bunch of other EU sovereign gov cloud products use it. Things i am unsure about:
|
|
I also like the general idea, the current notes editor seems very limited. But I would appreciate if there would be some easy migration path from markdown based notes to the new format, and a method to render the new notes format to reports. |
| return self.template.description | ||
|
|
||
|
|
||
| class Note( |
There was a problem hiding this comment.
Suggestion: Having a stable order identifier on the model might be interesting
There was a problem hiding this comment.
@matmair can you clarify this point? I'm not sure what you mean here
There was a problem hiding this comment.
Having a field that keeps the order of the notes for that object. That could be used to set a stable order of notes in the UI. Right now the order depends on the creation order/pk if I interpret it right.
| self.check_delete() | ||
| super().delete() | ||
|
|
||
| def clean(self): |
There was a problem hiding this comment.
If possible I would like to see security relevant cleaning data outside of a model class and tested independently so that missing coverage is more visible in the reports
There was a problem hiding this comment.
Can you propose what that would look like?
|
Can we remove django-markdownify with this? would remove a few dependencies. I have made a small demo branch for this: matmair#676 |
Co-authored-by: Matthias Mair <code@mjmair.com>
- Only used for this migration - Will potentially be removed at some point in the future?
3 participants
Example Screenshots
Generic Notes Table
The major item here is to move all notes into a new table, which has generic FK links to other items. This brings it into line with how we handle attachments, parameters, etc.
Multiple Notes
We can now specify multiple, separate notes per item. This is useful for keeping different types of notes separate - e.g. shipping instructions vs inspection instructions (for e.g.)
Render to report
Rendering notes to reports is now greatly improved. In particular, images are correctly handled now - and can even be resized based on the specified size in the HTML content
Editor Updates
Also introduces the possibility of a more intuitive notes editor.
At this stage, I am trialing BlockNote - which provides navite mantine support, and provides a "notion style" editing interface.I am using the Rich Text Editor shipped as part of the Mantine framework.
Pros:
Cons:
Tasks
Ideas for future work