Skip to content
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

add option to remove all fields within a connection type section #3381

Conversation

christianvogt
Copy link
Contributor

https://issues.redhat.com/browse/RHOAIENG-14752

Description

When removing a section, the modal now includes an option to also remove all fields associated to the section; if there are any. Selecting the checkbox will remove the listed fields, otherwise only the section itself is removed.

Also fixed an issue where duplicating a field would add it to the end of the list instead of immediately adjacent to the field you duplicated. This also makes it easier to quickly create fields for testing this issue.

image

image
Note that the spacing between the list and the expandable title is the PF default behavior.

How Has This Been Tested?

  • enable the connection types feature flag disableConnectionTypes
  • Navigate to Settings -> Connection types as an admin
  • create a connection type
  • Add various sections and fields
  • Delete sections choosing at times to delete the associated fields or not

Test Impact

Added unit tests.

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

cc @simrandhaliw

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

Attention: Patch coverage is 52.94118% with 16 lines in your changes missing coverage. Please review.

Project coverage is 85.05%. Comparing base (d9788f8) to head (c1ebdd1).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...onTypes/manage/ManageConnectionTypeFieldsTable.tsx 0.00% 10 Missing ⚠️
frontend/src/concepts/connectionTypes/utils.ts 50.00% 3 Missing ⚠️
...ypes/manage/ManageConnectionTypeFieldsTableRow.tsx 50.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3381      +/-   ##
==========================================
- Coverage   85.08%   85.05%   -0.04%     
==========================================
  Files        1329     1330       +1     
  Lines       29931    29951      +20     
  Branches     8200     8204       +4     
==========================================
+ Hits        25466    25474       +8     
- Misses       4465     4477      +12     
Files with missing lines Coverage Δ
...nd/src/concepts/dashboard/DashboardModalFooter.tsx 100.00% <ø> (ø)
...ypes/manage/ConnectionTypeDataFieldRemoveModal.tsx 20.00% <100.00%> (ø)
...ctionTypes/manage/ConnectionTypeFieldMoveModal.tsx 100.00% <ø> (ø)
...nectionTypes/manage/ConnectionTypeSectionModal.tsx 100.00% <ø> (ø)
...nTypes/manage/ConnectionTypeSectionRemoveModal.tsx 100.00% <100.00%> (ø)
frontend/src/concepts/connectionTypes/utils.ts 90.64% <50.00%> (-1.84%) ⬇️
...ypes/manage/ManageConnectionTypeFieldsTableRow.tsx 84.31% <50.00%> (ø)
...onTypes/manage/ManageConnectionTypeFieldsTable.tsx 48.33% <0.00%> (-10.93%) ⬇️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9788f8...c1ebdd1. Read the comment docs.

Copy link
Contributor

@emilys314 emilys314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frontend/src/pages/connectionTypes/manage/ConnectionTypeFieldRemoveModal.tsx is no longer being used since the datafield and section field have separate modals. I think it can be deleted

@emilys314
Copy link
Contributor

everything seems to be working good

@christianvogt
Copy link
Contributor Author

@emilys314 Gone ahead and removed that file (it actually shows up as a rename now in the change) and also rebased.

@emilys314
Copy link
Contributor

/lgtm

Copy link
Contributor

openshift-ci bot commented Oct 25, 2024

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 988ed4c into opendatahub-io:main Oct 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants