-
Notifications
You must be signed in to change notification settings - Fork 43
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
✨ Archetype form for Create and Edit #1343
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1343 +/- ##
==========================================
+ Coverage 42.39% 42.40% +0.01%
==========================================
Files 137 137
Lines 4291 4292 +1
Branches 1007 1008 +1
==========================================
+ Hits 1819 1820 +1
Misses 2460 2460
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
1614192
to
9356f9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is in great shape and would be happy to merge it at this stage if we want to push the remaining open tasks to later PRs.
name: string; | ||
description?: string; | ||
comments?: string; | ||
criteriaTags: string[]; // TODO: string[] only works if tags are uniquely named globally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wrong about this field needing to be surface on UI level. This is an internal hub construct. We only need to pass tags as they are passed on applications today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Humm -- I still think that the 2 tag files are needed on UI...
- Criteria, so people can choose the tags that go in to the archetype-application matching algo
- "Static" archetype, so people can choose tags that should be associated with applications just by being matched to the archetype
There is a difference between what types the form values have and what is in the data model. Currently, the Autocomplete
component only supports a single list of strings for everything. That doesn't line up nicely with an array of Tags or similar. Thankfully when I tested creating Tags from curl, the names did need to be globally unique (regardless of what tag category tags belong to), so the current impl of Autocomplete is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sjd78 I think the hub is taking care of that logic for us. It should work similarly to how the application form works. Can just pass tags as they are. The hub should know which tag belongs to what category & what tags are used in the matching logic.
As far as the autocomplete component goes, I think we should just show the list of tags with no category for this first pass. And address the missing visual component of showing the associated category after feature freeze.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. The form will display whatever data is provided for an edit and just empty values for a new archetype.
The form interface uses a string[]
instead of a TagRef[]
so the Autocomplete
component is easier to use (via the ItemsSelect
wrapper component).
As for the "criteria tags" and "archetype tags" fields, they are both still required inputs on the enhancement: https://github.com/konveyor/enhancements/blob/90b827b68cc367284a66bf66f087d5c263487e05/enhancements/assessment-module/README.md#creating-new-archetypes
Are you talking about swapping the actual Archetype API model from Tag[]
to TagRef[]
?
client/src/app/pages/archetypes/components/archetype-form/archetype-form.tsx
Outdated
Show resolved
Hide resolved
I'd like to throw the Stakeholders in there with the autocomplete and then move forward. That shouldn't take me much more time today after sorting out how to use Autocomplete for tags. |
9356f9b
to
684136b
Compare
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
4ec2096
to
50812df
Compare
Add the Archetype form along with Create and Edit actions from the archetype table.
MSW stubs for the hub archetype api are enabled by default for now.
Resolves #1265
Screenshots
Create new (empty):
Tag selection field open with typeahead active:
Create fully filled in:
Create in error with an archetype name that already exists:
Edit existing: