Closed PaulCCCCCCH closed 1 year ago
I wanted to test this locally to see what the interface looks like, but wasn't able to test it due to the following error when clicking on the "Systems" tab. Are you able to run this code still?
I think this is something that has been fixed on the main branch (3e6be75 change analysis description to be nullable (#452)
). We probably just need to merge main in to make it work.
I don't seem to have this problem locally (maybe it's caused by some private systems that I don't see). I have merged main into this branch. Is it working now?
Hi @PaulCCCCCCH , just wanted to check if this is ready for me to review/approve. I see one remaining comment from Lyuyang.
Hi @PaulCCCCCCH , just wanted to check if this is ready for me to review/approve. I see one remaining comment from Lyuyang.
I'm still testing my change. Will push an update soon.
There is a potential issue for this PR:
The existence of a system without system_tags
field will break the backend because system_tags
is now a required field. After merging it into main, we have to migrate the DB to add a default value for system_tags
, and we have to do the migration whenever a user submits a system with an older version. This should be OK for prod environment because we always deploy the latest version.
Part of issue #357
Now users can now add tags to their systems. We can support filtering in another PR.
We have discussed about tagging each individual system in the meeting. We are yet to decide whether to implement it or not, so I put a TBD in issue #357.
By the way, I noticed that we could use the exact same implementation for
Shared Users
andMetrics
to implement the tags. It is easy and makes the style more consistent. the only thing is, you cannot double click and edit added tags. Any thoughts?Screenshots:
Submission Page
In System List