🐛 Applications form: Fix contributors and tags field handling #1408
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `contributors` field on the `Application` payload needs to be
a pure `Ref` object or it will be rejected by the REST API call.
Adopt the same set of data transforms used in the archetype-form to
handle getting the correct set of data.
Related changes:
- REST `createApplication()` function updated to have the proper
return type (response data is not unwrapped)
- Query `useCreateApplicationMutation()` updated to properly pass the
newly created `Application` to the `onSuccess()` handler
- `onCreateApplicationSuccess()` in the application form updated to
use the correct onSuccess() response data
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Followup konveyor#1404 to update the way tags are updated to an existing application. Tags that are from an analysis need to be included with the update payload or they will be removed. This is different behavior from archetype or assessment tags. No updates to an application can remove archetype or assessment tags. Summary of changes and refactoring: - As form values, keep the tags using just the tag name as a string - Moved all data access/mutation code to hook `useApplicationFormData()` to logically divide concerns (data access v. UI handling) - Tags dropdown will only display/edit the application's "Manual tags" - `onSubmit()`'s payload building simplified using partially curried helper functions - Migrate to use `ItemsSelect` for handling the Tags Update `useUpdateApplicationMutation()` to provide the mutation's payload to the `onSuccess()` function. This allows the `onSuccess()` function to toast a message with the application's name. Add utility functions to `model-utils.tsx`: - convert objects that look like `Ref` object into exactly a `Ref` object: - toRef() - toRefs() - Match a set of items that look like `Ref` objects against a (set of) values and the matching matches as exactly `Ref` objects: items into exactly `Ref` objects: - matchItemsToRef() - matchItemsToRefs() Signed-off-by: Scott J Dickerson <[email protected]>
sjd78
force-pushed
the
contrib_applications_form
branch
from
43a14f4 to
d8d5cb8
Compare
sjd78
changed the title
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1408 +/- ##
==========================================
+ Coverage 41.01% 41.24% +0.23%
==========================================
Files 137 138 +1
Lines 4333 4347 +14
Branches 1045 1005 -40
==========================================
+ Hits 1777 1793 +16
- Misses 2469 2542 +73
+ Partials 87 12 -75
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
ibolton336
reviewed
Sep 28, 2023
| @@ -683,3 +583,95 @@ export const ApplicationForm: React.FC<ApplicationFormProps> = ({ | |||
| </Form> | |||
| ); | |||
| }; | |||
|
|
|||
| const useApplicationFormData = ({ | |||
ibolton336
reviewed
Sep 28, 2023
| formValues.contributors.includes(stakeholder.name) | ||
| ); | ||
| // Note: We need to manually retain the tags with source != "" in the payload | ||
| const tags = [...(tagsToRefs(formValues.tags) ?? []), ...nonManualTags]; |
ibolton336
reviewed
Sep 28, 2023
| name: formValues.name.trim(), | ||
| description: formValues.description.trim(), | ||
| comments: formValues.comments.trim(), | ||
| businessService: matchingBusinessService | ||
|
|
||
| businessService: businessServicesToRef(formValues.businessServiceName), |
There was a problem hiding this comment.
Looks great - I found this util function naming to be slightly confusing since it is essentially looking to match one business service name to its matching ref. Would businessServiceToRef make more sense? It is probably a toss-up just wanted your thoughts.
There was a problem hiding this comment.
Yeah that would make more sense to go singular throughout
ibolton336
reviewed
Sep 28, 2023
| @@ -439,40 +360,19 @@ export const ApplicationForm: React.FC<ApplicationFormProps> = ({ | |||
| )} | |||
| /> | |||
|
|
|||
| <HookFormPFGroupController | |||
| <ItemsSelect<Tag, FormValues> | |||
ibolton336
reviewed
Sep 28, 2023
| // Fetch data | ||
| const { tagCategories } = useFetchTagCategories(); | ||
| const tags = useMemo( | ||
| () => tagCategories.flatMap((tc) => tc.tags).filter(Boolean) as Tag[], |
There was a problem hiding this comment.
Is there any way to let typescript in on this .filter(Boolean) magic?
There was a problem hiding this comment.
Ohhhh, just found this nugget: https://www.karltarvas.com/typescript-array-filter-boolean.html
Dropped a file in client/src/types with the extra definition and it's working! 😁
There was a problem hiding this comment.
Oh my..... that is a beautiful thing!
ibolton336
requested changes
Sep 28, 2023
Signed-off-by: Scott J Dickerson <[email protected]>
ibolton336
approved these changes
Sep 28, 2023
|
The tags update portion of this PR was raised in the referenced jira issue. |
sjd78
added a commit
to sjd78/tackle2-ui
that referenced
this pull request
Oct 3, 2023
Resolves: konveyor#1410 Follows up: konveyor#1408 - The application form will only show manual tags (i.e. source="") - All tags are allowable for selection as a manual tag - Change the label on the field from "Tags" to "Manual Tags" to match the "Manual" grouping of tags in the application drawer Signed-off-by: Scott J Dickerson <[email protected]>
ibolton336
pushed a commit
that referenced
this pull request
Oct 3, 2023
Resolves: #1410 Follows up: #1408 - The application form will only show manual tags (i.e. source="") - All tags are allowable for selection as a manual tag - Change the label on the field from "Tags" to "Manual Tags" to match the "Manual" grouping of tags in the application drawer Signed-off-by: Scott J Dickerson <[email protected]>
sjd78
added a commit
to sjd78/tackle2-ui
that referenced
this pull request
Oct 5, 2023
With hub changes to combine the archetype and non-archetype tags into a single field, change the form processing such that the non-archetype fields are ignored but still sent on an update. This change mirrors the behavior of manual / non-manual tags on applications and the application form. Stakeholder and stakeholder groups handling is updated to use the `*ToRef()` helper function style introduced in the application form change konveyor#1408. Signed-off-by: Scott J Dickerson <[email protected]>
sjd78
added a commit
to sjd78/tackle2-ui
that referenced
this pull request
Oct 5, 2023
With hub changes to combine the archetype and non-archetype tags into a single field, change the form processing such that the non-archetype fields are ignored but still sent on an update. This change mirrors the behavior of manual / non-manual tags on applications and the application form. Stakeholder and stakeholder groups handling is updated to use the `*ToRef()` helper function style introduced in the application form change konveyor#1408. Signed-off-by: Scott J Dickerson <[email protected]>
ibolton336
pushed a commit
that referenced
this pull request
Oct 6, 2023
…1438) ### Summary Resolves: #1436 With hub updates to tag handling and api field name changes related to archetypes, adjustments needed to be made to the UI. ### Archetype model - Update `models.ts` to the current state of `TagRef` and `Archetype` in terms of field names and data types. - MSW stub data updated to use api model changes. ### Archetype Table - **Note**: The tags column on the archetype table is currently displaying ALL tags returned with no source differentiation. ### Archetype Add/EditForm - Update `ArchetypeForm` to use `criteria` instead of `criteriaTags` as per the api model changes. - With hub changes to combine the archetype and non-archetype tags into a single field, the form processing now ignored non-archetype fields but still sends them on an update. - Note: This change mirrors the behavior of manual / non-manual tags on applications and the application form. - Stakeholder and stakeholder groups handling is updated to use the `*ToRef()` helper function style introduced in the application form change #1408. ### Archetype Detail Drawer - Since all tags for an archetype are provided in the same field `tags`, filter them into "archetype tags" (i.e. an empty source) and "assessment tags" (i.e. have a non-empty source) to display them separately. - Note: The "archetype tags" are the tags manually attached to the archetype in the new/edit form. --------- Signed-off-by: Scott J Dickerson <[email protected]>
1 task
This was referenced Nov 22, 2023
This was referenced Jan 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contributors field
The
contributorsfield on theApplicationpayload needs to be a pureRefobject or it will be rejected by the REST API call. Adopt the same set of data transforms used in the archetype-form to handle getting the correct set of data.Related changes (came up when working on #1331 that ended up revealing #1404):
REST
createApplication()function updated to have the proper return type (response data is not unwrapped)Query
useCreateApplicationMutation()updated to properly pass the newly createdApplicationto theonSuccess()handleronCreateApplicationSuccess()in the application form updated to use the correctonSuccess()response dataResolves: #1404
Tags field
Followup #1403 to update the way tags are updated to an existing application. Tags that are from an analysis need to be included with the update payload or they will be removed. This is different behavior from archetype or assessment sourced tags. No updates to an application can remove archetype or assessment tags.
Summary of changes and refactoring:
useApplicationFormData()to logically divide concerns (data access v. UI handling)onSubmit()'s payload building simplified using partially curried helper functionsItemsSelectfor handling the TagsUpdate
useUpdateApplicationMutation()to provide the mutation's payload to theonSuccess()function. This allows theonSuccess()function to toast a message with the application's name.Add utility functions to
model-utils.tsx:convert objects that look like
Refobject into exactly aRefobject:Match a set of items that look like
Refobjects against a (set of) values and return the matching items as exactlyRefobjects: