Skip to content

feat: proper listing of project data connectors #3528

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Mar 21, 2025
Merged

feat: proper listing of project data connectors #3528

merged 15 commits into from
Mar 21, 2025

Conversation

olevski
Copy link
Member

@olevski olevski commented Feb 11, 2025 •
edited
Loading

/deploy renku-data-services=build-project-authz-disentangle

@olevski
Copy link
Member Author

olevski commented Feb 11, 2025

This change is part of the following stack:

Change managed by git-spice.

@olevski olevski mentioned this pull request Feb 11, 2025
Merged
@olevski olevski force-pushed the feat-data-connector-listing branch from 2a71a63 to b8a5a71 Compare February 14, 2025 12:55
Base automatically changed from feat-create-data-connector-in-project to build-project-authz-disentangle February 19, 2025 09:23
@olevski olevski marked this pull request as ready for review March 18, 2025 16:46
@olevski olevski requested a review from a team as a code owner March 18, 2025 16:46
@olevski olevski temporarily deployed to renku-ci-ui-3528 March 18, 2025 16:46 — with GitHub Actions Inactive
@olevski olevski marked this pull request as draft March 18, 2025 16:47
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-3528.dev.renku.ch

@olevski olevski force-pushed the feat-data-connector-listing branch from cb634f7 to cce5b4f Compare March 19, 2025 01:29
@olevski olevski force-pushed the build-project-authz-disentangle branch from f600423 to 544ea24 Compare March 20, 2025 12:22
@olevski olevski force-pushed the feat-data-connector-listing branch from cce5b4f to daf04a4 Compare March 20, 2025 12:22
@olevski olevski marked this pull request as ready for review March 20, 2025 12:31
@olevski olevski temporarily deployed to renku-ci-ui-3528 March 20, 2025 12:36 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi self-assigned this Mar 20, 2025
@olevski
Copy link
Member Author

olevski commented Mar 20, 2025

FYI the first 3 commits of this PR have already been reviewed in #3525

lorenzo-cavazzi
lorenzo-cavazzi requested changes Mar 20, 2025
View reviewed changes
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Very nice! 👏

The feature work well and the code looks great! I dropped a few comments but it's mostly minor changes.

As discussed offline, the Select dropdown shows all elements with a grey background. There should be a function to match the currently selected element, and it probably returns true for every entry.

image

client/src/features/dataConnectorsV2/components/DataConnectorsBoxListDisplay.tsx Outdated
Comment on lines 161 to 165
{visibilityWarning && (
<div>
<DataConnectorNotVisibleToAllUsersBadge />
</div>
)}
Copy link
Member

Choose a reason for hiding this comment

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

NitpicK: this works fine, but I wonder if it's best position next to the visibility. Something like this

image

Copy link
Member Author

Choose a reason for hiding this comment

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

I think @lokijuhy wanted it below from the notes she made on notion.
https://www.notion.so/renku/Project-Disentanglement-Build-Team-17e0df2efafc80fcbaa6f0b363a574f0?pvs=4#19e0df2efafc80e6ae19ce47cef02770

But yeah I dont really care I will move it back up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in c2f19b2

@olevski
Copy link
Member Author

olevski commented Mar 20, 2025

Here is the followup issue for the namespace selection when creating a data connector:
#3606

@olevski @lorenzo-cavazzi
squashme: Apply suggestions from code review
e15deb9 
Co-authored-by: Lorenzo Cavazzi <43481553+lorenzo-cavazzi@users.noreply.github.com>
@olevski olevski temporarily deployed to renku-ci-ui-3528 March 20, 2025 17:57 — with GitHub Actions Inactive
@olevski olevski temporarily deployed to renku-ci-ui-3528 March 20, 2025 18:07 — with GitHub Actions Inactive
@olevski olevski temporarily deployed to renku-ci-ui-3528 March 20, 2025 23:17 — with GitHub Actions Inactive
@olevski
Copy link
Member Author

olevski commented Mar 20, 2025

The dropdown styling is back to normal.

Screenshot From 2025-03-21 00-50-49

@olevski olevski requested a review from lorenzo-cavazzi March 20, 2025 23:51
lorenzo-cavazzi
lorenzo-cavazzi approved these changes Mar 21, 2025
View reviewed changes
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Lgtm! 🚀
Very nice contribution to the UI 🙌

As discussed offline, I pushed a commit to improve consistency in spacing. It's a few changes in Bootstrap spacing classes d82ce87

@olevski olevski merged commit e0426cd into build-project-authz-disentangle Mar 21, 2025
11 of 12 checks passed
@olevski olevski deleted the feat-data-connector-listing branch March 21, 2025 09:58
@RenkuBot
Copy link
Contributor

Tearing down the temporary RenkuLab deplyoment for this PR.

olevski added a commit that referenced this pull request Mar 21, 2025
@olevski @lorenzo-cavazzi
feat: proper listing of project data connectors (#3528)
285dddf 
Co-authored-by: Lorenzo Cavazzi <43481553+lorenzo-cavazzi@users.noreply.github.com>
Co-authored-by: Lorenzo <lorenzo.cavazzi.tech@gmail.com>
olevski added a commit that referenced this pull request Mar 24, 2025
@olevski @lorenzo-cavazzi
feat: proper listing of project data connectors (#3528)
caaaccc 
Co-authored-by: Lorenzo Cavazzi <43481553+lorenzo-cavazzi@users.noreply.github.com>
Co-authored-by: Lorenzo <lorenzo.cavazzi.tech@gmail.com>
olevski added a commit that referenced this pull request Mar 24, 2025
@olevski @lorenzo-cavazzi
feat: create data connectors in project (#3525) (#3601)
fbdd160 
* feat: create data connector in project (#3525)

* feat: proper listing of project data connectors (#3528)

* fix: multi slug regex backtracking

---------

Co-authored-by: Lorenzo <lorenzo.cavazzi.tech@gmail.com>
leafty pushed a commit that referenced this pull request Apr 22, 2025
@olevski @lorenzo-cavazzi @leafty
feat: create data connectors in project (#3525) (#3601)
cc5913a 
* feat: create data connector in project (#3525)

* feat: proper listing of project data connectors (#3528)

* fix: multi slug regex backtracking

---------

Co-authored-by: Lorenzo <lorenzo.cavazzi.tech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorenzo-cavazzi lorenzo-cavazzi lorenzo-cavazzi approved these changes

Assignees

@lorenzo-cavazzi lorenzo-cavazzi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@olevski @RenkuBot @lorenzo-cavazzi