-
Notifications
You must be signed in to change notification settings - Fork 324
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
Remove listGroups while building categories #11988
base: develop
Are you sure you want to change the base?
Conversation
🧪 Storybook is successfully deployed!📊 Dashboard:
|
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.
CR ✅
# Conflicts: # app/gui/src/dashboard/hooks/backendHooks.tsx # app/gui/src/dashboard/layouts/AssetsTable.tsx
Re-requesting CR as this PR changed after the latest code review |
Previosly we were wrongly relying on the presence of the file. That way, a bash script meant that NI integration was wrongly used. This change uses Tika, but it has already been present in other subprojects so no additional dependency shall be added. Follow up on #11880.
@@ -4495,10 +4495,12 @@ lazy val `runtime-version-manager` = project | |||
libraryDependencies ++= Seq( | |||
"com.typesafe.scala-logging" %% "scala-logging" % scalaLoggingVersion, | |||
"org.apache.commons" % "commons-compress" % commonsCompressVersion, | |||
"org.apache.tika" % "tika-core" % tikaVersion, |
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.
why is this part of the changeset?
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.
Shouldn't be. I think it's a bad merge or something. Will fix, thank you!
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.
CR ✅
would like to get confirmation that we want to remove the manage permissions modal though
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.
No changes to project-manager
, its legal
, lib/scala
, please.
@@ -0,0 +1,8 @@ | |||
|
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.
No changes to project-manager
in this PR please.
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.
Yeah, this was a bad merge or something
Pull Request Description
This PR adds new field called
groups
on the User object to avoid fetching the usergroups list to get the names of the groups.Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.