-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-14319: Create initial project structure for new NiFi Registry UI #9855
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
Conversation
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.
Thanks for this work on NiFi Registry @sardell!
The standard ci-workflow
builds are failing, does this build locally for you, or is this still a work in progress that should be marked as draft?
@exceptionfactory Thanks for raising the failing CI to my attention. It looks like the failure was due to the favicon missing from the old registry ui directory. I accidentally deleted it when copying over images to the new project. I noticed the missing file while working on this PR and added the file back, but it seems that file was accidentally added to the .gitignore (I think intention of that .gitignore line was to ignore the generated favicon, not all instances of that file). I believe this is why I could successfully build locally and also see the CI failures you pointed out. Fingers crossed the CI passes this time. I'll keep an eye on it. UPDATE: Looks like the CI build is now failing on a build step that runs fine locally. Will need to investigate a little further. |
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.
Thanks for the update @sardell, although the build completes on macOS, it is failing on Linux and Windows.
On Linux, there appears to be something platform-dependent:
[INFO] ::group::❌ > nx run shared:build:production
[INFO]
[INFO] Building Angular Package
[INFO]
[INFO] ------------------------------------------------------------------------------
[INFO] Building entry point '@nifi/shared'
[INFO] ------------------------------------------------------------------------------
[INFO] - Compiling with Angular sources in Ivy full compilation mode.
[INFO] ✖ Compiling with Angular sources in Ivy full compilation mode.
[INFO]
[INFO] NX The package "@esbuild/linux-x64" could not be found, and is needed by esbuild.
bd6fd83
to
74df539
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.
looks good for the 1st glance, I post my first comments, will continue the review soon
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/header.component.ts
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/state/droplets/droplets.effects.ts
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/state/droplets/droplets.effects.ts
Outdated
Show resolved
Hide resolved
...olorer/feature/ui/import-new-flow-version-dialog/import-new-flow-version-dialog.component.ts
Outdated
Show resolved
Hide resolved
...ry/src/app/pages/expolorer/feature/ui/droplet-table-filter/droplet-table-filter.component.ts
Outdated
Show resolved
Hide resolved
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.
@sardell thanks for the PR! I have left several comments on things that need to be addressed. Apologies that there are so many comments as I know this is a big undertaking and your effort here is appreciated. I am sure I will have more comments after you make the requested changes.
Also, I didn't have a great place to leave the following comments so placing them here:
Please run prettier and lint:fix. There are several strange formatting issues.
Please delete nifi-registry/public directory and the favicon.ico.
nifi-frontend/src/main/frontend/apps/nifi-registry/project.json
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/.eslintrc.json
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/.eslintrc.json
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/.eslintrc.json
Outdated
Show resolved
Hide resolved
...rc/app/pages/expolorer/feature/ui/delete-droplet-dialog/delete-droplet-dialog.component.html
Outdated
Show resolved
Hide resolved
...rc/app/pages/expolorer/feature/ui/delete-droplet-dialog/delete-droplet-dialog.component.html
Outdated
Show resolved
Hide resolved
...rc/app/pages/expolorer/feature/ui/delete-droplet-dialog/delete-droplet-dialog.component.html
Outdated
Show resolved
Hide resolved
...rc/app/pages/expolorer/feature/ui/delete-droplet-dialog/delete-droplet-dialog.component.html
Outdated
Show resolved
Hide resolved
...rc/app/pages/expolorer/feature/ui/delete-droplet-dialog/delete-droplet-dialog.component.html
Outdated
Show resolved
Hide resolved
@scottyaslan @elcsiga Thank you both for taking the time to review this PR! I've addressed all your comments. When you have a chance, could you please take another look? @scottyaslan I've updated the Jira tickets for the Registry UI work to be tasks organized under an epic per your suggestion. I've also started breaking down the work into much smaller chunks (see the ticket for the buckets view and its subtasks for an example). I'll continue this work for the users and groups view. |
@scottyaslan @elcsiga Could you please re-review when you have time? |
Reviewing... |
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.
Hey @sardell! I have reviewed this PR and it is looking good. There are still several things to get dialed in but all in all this is headed in the right direction. Please let me know if you have any questions or comments or if you need any clarity on my latest set of comments.
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/header.component.scss
Outdated
Show resolved
Hide resolved
...ifi-registry/src/app/ui/header/common/context-error-banner/context-error-banner.component.ts
Show resolved
Hide resolved
...orer/feature/ui/import-new-flow-version-dialog/import-new-flow-version-dialog.component.html
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/droplets.service.ts
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/droplets.service.ts
Show resolved
Hide resolved
.../app/pages/expolorer/feature/ui/import-new-flow-dialog/import-new-flow-dialog.component.html
Outdated
Show resolved
Hide resolved
...src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/explorer.component.html
Outdated
Show resolved
Hide resolved
...d/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/explorer.component.ts
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/header.component.ts
Outdated
Show resolved
Hide resolved
nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/state/error/error.actions.ts
Outdated
Show resolved
Hide resolved
@sardell also please run |
...src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/explorer.component.html
Show resolved
Hide resolved
...src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/explorer.component.html
Outdated
Show resolved
Hide resolved
@sardell I hope to get back to reviewing this next week. Sorry for the delay! |
...rc/main/frontend/apps/nifi-registry/src/app/pages/resources/feature/resources.component.html
Outdated
Show resolved
Hide resolved
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.
@sardell sorry for the delay in getting back to this. Thanks for the updates! I think we are close. Please rebase and I left just a few more comments. Mostly, I would like to see the route be renamed back to 'explorer' and get rid of the queryParams.
@scottyaslan Your latest round of feedback has been addressed. Please take another look whenever you have time. Thanks! |
751d1a1
to
e1fe7ce
Compare
20abbf2
to
aab923e
Compare
* Linux solution: https://github.com/evanw/esbuild/issues/1819\#issuecomment-2426254293 * Windows solution: nrwl/nx#29551
* fix error service
* fix multiple namings * template cleanup * configuration changes to align better with NiFi * style fixes
* use px units instead of calc for logo container * use unique contexts for dialog errors * update explorer page title * margin and sizing class updates * run prettier to fix formatting issues
* remove rej file * remove unused deps * add standalone to components missing that configuration * hadle legacy routes
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.
Thanks @sardell this is a great start!
Summary
NIFI-14319
Description
This PR establishes the initial project setup for the new NiFi Registry UI and includes the new explorer view.
Testing
Build and start the old registry project with maven using the existing instructions. Next, run
npm ci
atnifi-frontend/src/main/frontend
and start the app by runningnpx nx run nifi-registry:serve
. Visithttp://localhost:4204/nifi-registry/
to see the new explorer page. Compare the old explorer view with the new one, ensuring feature parity between the two views. Run unit tests withnpx nx run nifi-registry:test
.NOTE: buckets cannot be created through the new UI yet, so in order to test the functionality of the new explorer view you must first create a bucket through the old UI.
Considerations
Styling - The old logo was created with a much darker background color in mind. It may be time for a refreshed logo, but for now I simply added the old one to the header along with a heading containing the application's name.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
LICENSE
andNOTICE
filesDocumentation