frontend: Add create resource UI #1996
Open
+1,435
−211
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
skoeva
force-pushed
the
create-resource-ui
branch
6 times, most recently
from
d1de90b
to
fe2cec7
Compare
|
These are some nice set of changes thanks @skoeva I was wandering what all resources we are covering to have the create resource button on?? And what is the reasoning behind adding it to some and not to others if that's the case? |
@ashu8912 Currently covering config maps, secrets, leases, and runtime classes as those I already found with base templates in their associated directories, though if needed, we can add templates for more resources and cover those as well |
illume
reviewed
Jun 19, 2024
There was a problem hiding this comment.
Nice work!
I left a few notes for your consideration.
As well as a CreateResourceButton.stories.tsx, maybe a few small pieces could be unit tested, and perhaps an e2e test would be useful for the overall feature?
(ps. it also needs rebasing against main now)
(pps. good work on adding the kube-ish function, we should validate the data matches what we're expecting a bit more... especially since we need to fix a bunch of types still)
Probably you know this already... but the k8s website gives example yaml for each resource in their docs. But adding defaults for all the resources could be left for a future PR I reckon... it shouldn't block merging IMHO. |
skoeva
force-pushed
the
create-resource-ui
branch
2 times, most recently
from
4fda387
to
fe2cec7
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
skoeva
force-pushed
the
create-resource-ui
branch
5 times, most recently
from
2af16dd
to
205914c
Compare
skoeva
force-pushed
the
create-resource-ui
branch
from
2886872
to
51b00c8
Compare
joaquimrocha
requested changes
Jul 3, 2024
skoeva
force-pushed
the
create-resource-ui
branch
3 times, most recently
from
ccb85e4
to
5268668
Compare
joaquimrocha
requested changes
Jul 11, 2024
skoeva
force-pushed
the
create-resource-ui
branch
4 times, most recently
from
5536809
to
ae5828a
Compare
skoeva
force-pushed
the
create-resource-ui
branch
from
cf4f9b1
to
e7d0024
Compare
|
Backend Code coverage changed from 53.2% to 53.4%. Change: .2% 😃. |
illume
requested changes
Sep 11, 2024
skoeva
force-pushed
the
create-resource-ui
branch
2 times, most recently
from
c5e93f8
to
5f5599f
Compare
|
Backend Code coverage changed from 60.2% to 60.4%. Change: .2% 😃. |
skoeva
force-pushed
the
create-resource-ui
branch
2 times, most recently
from
ddd6c4a
to
664e8c1
Compare
1 task
skoeva
force-pushed
the
create-resource-ui
branch
from
664e8c1
to
63cb5cf
Compare
joaquimrocha
requested changes
Sep 20, 2024
|
|
||
| const newItemDefs = obj!; | ||
|
|
||
| if (!applyOnSave) { |
There was a problem hiding this comment.
Shouldn't this be if applyOnSave is true? We are calling the apply function.
There was a problem hiding this comment.
This is essentially checking whether or not to use an external dispatch (formerly called externalDispatch). The name might be a bit confusing so I think it could be changed to something closer to what it was originally, but setting this to true means no dispatch is performed when creating a resource and 2 dispatches are performed when editing
frontend/src/lib/k8s/secret.ts
Outdated
Comment on lines
13
to
18
| static getBaseObject(): KubeSecret { | ||
| const baseObject = super.getBaseObject() as KubeSecret; | ||
| return { | ||
| ...baseObject, | ||
| kind: Secret.kind, | ||
| metadata: { ...baseObject.metadata, name: 'base-secret' }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
I was hoping we could make things work without having to override this function everywhere (but only in cases where e.g. the objectName is not matching the kind).
There was a problem hiding this comment.
I imagine it makes more sense to keep the attributes (metadata, etc.) in each resource's respective file (meaning keeping this function override) over consolidating it all somehow in cluster.ts?
There was a problem hiding this comment.
What I mean is that I think there should be a way for us to have this logic abstract enough in the cluster.ts that it adapts correctly to most of the subclasses. We should definitely not move individual resources' logic into the cluster.ts file.
skoeva
force-pushed
the
create-resource-ui
branch
from
63cb5cf
to
4e3dbb1
Compare
dosubot
bot
added
the
size:XXL
Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
skoeva
force-pushed
the
create-resource-ui
branch
from
4e3dbb1
to
134179c
Compare
These changes introduce a new UI feature that allows users to create resources from the associated list view. Clicking the 'Create' button opens up the EditorDialog used in the generic 'Create / Apply' button, now accepting generic YAML/JSON text rather than explicitly expecting an item that looks like a Kubernetes resource. The dialog box also includes a generic template for each resource. The apply logic for this new feature (as well as the original 'Create / Apply' button) has been consolidated in EditorDialog, with a flag allowing external components to utilize their own dispatch functionality. Fixes: #1820 Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
skoeva
force-pushed
the
create-resource-ui
branch
6 times, most recently
from
6f3ad30
to
88d8c26
Compare
|
Backend Code coverage changed from 60.1% to 60.3%. Change: .2% 😃. Coverage report |
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.
These changes introduce a new UI feature that allows users to create resources from the associated list view. Clicking the 'Create' button opens up the
EditorDialogused in the generic 'Create / Apply' button, now accepting generic YAML/JSON text rather than explicitly expecting an item that looks like a Kubernetes resource. The dialog box also includes a generic template for each resource. The apply logic for this new feature (as well as the original 'Create / Apply' button) has been consolidated in EditorDialog, with a flag allowing external components to utilize their own dispatch functionality.Fixes: #1820
Testing
Create resource button
cd app & npm start) and navigate to a resource (e.g. Config Maps)EditorDialog changes
cd app & npm start) and click the 'Create / Apply' buttonScreenshots