You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/08/06 13:04:48 UTC
[GitHub] [apisix-dashboard] 1328735450 opened a new pull request, #2573: fix/plugin-dwarer-component
1328735450 opened a new pull request, #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573
Please answer these questions before submitting a pull request, **or your PR will get closed**.
**Why submit this pull request?**
- [x] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
**What changes will this PR take into?**
Please update this section with detailed description.
**Related issues**
fix/resolve #2572
**Checklist:**
- [ ] Did you explain what problem does this PR solve? Or what new features have been added?
- [ ] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [ ] Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r939845765
##########
web/src/components/Plugin/PluginPage.tsx:
##########
@@ -280,11 +284,10 @@ const PluginPage: React.FC<Props> = ({
const newPlugins = {
...initialData,
[name]: { ...monacoData, disable: !formData.disable },
+ ...(shouldDelete ? { [name]: null } : {}),
};
- if (shouldDelete === true) {
- newPlugins[name] = null;
- }
- onChange(newPlugins, form.getFieldValue('plugin_config_id'));
+ const handleType = shouldDelete ? 'delete' : 'edit';
+ onChange(newPlugins, handleType, form.getFieldValue('plugin_config_id'));
Review Comment:
Please check the code here, I think it is the data there that is having an effect, we may not need to pass the `initialData` data. The onChange logic here I don't think should be changed, but the source of the problem should be looked at.
https://github.com/apache/apisix-dashboard/blob/master/web/src/pages/Plugin/PluginMarket.tsx#L54
##########
web/src/components/Plugin/PluginPage.tsx:
##########
@@ -280,11 +284,10 @@ const PluginPage: React.FC<Props> = ({
const newPlugins = {
...initialData,
[name]: { ...monacoData, disable: !formData.disable },
+ ...(shouldDelete ? { [name]: null } : {}),
Review Comment:
As this component is used in a number of places, we need to be extra careful in modifying this part of the code
##########
web/src/pages/Plugin/List.tsx:
##########
@@ -131,6 +131,13 @@ const Page: React.FC = () => {
setVisible(false);
setName('');
ref.current?.reload();
+ notification.success({
+ message: `${formatMessage({
+ id: `component.global.${shouldDelete ? 'delete' : 'edit'}`,
+ })} ${formatMessage({
+ id: 'menu.plugin',
+ })} ${formatMessage({ id: 'component.status.success' })}`,
Review Comment:
This writing style will result in a Chinese state with extra spaces
##########
web/src/pages/Plugin/PluginMarket.tsx:
##########
@@ -48,14 +48,21 @@ const PluginMarket: React.FC = () => {
initialData={initialData}
type="global"
schemaType="route"
- onChange={(pluginsData) => {
+ onChange={(pluginsData, handleType) => {
createOrUpdate({
plugins: {
...initialData,
...pluginsData,
},
}).then(() => {
initPageData();
+ notification.success({
+ message: `${formatMessage({
+ id: `component.global.${handleType}`,
+ })} ${formatMessage({
+ id: 'menu.plugin',
+ })} ${formatMessage({ id: 'component.status.success' })}`,
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-commenter commented on pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#issuecomment-1207422958
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/2573?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2573](https://codecov.io/gh/apache/apisix-dashboard/pull/2573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cf482e9) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/adcd2e376635cad43c3518ac3ebabde4b6c2b854?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (adcd2e3) will **decrease** coverage by `10.58%`.
> The diff coverage is `50.00%`.
```diff
@@ Coverage Diff @@
## master #2573 +/- ##
===========================================
- Coverage 75.92% 65.33% -10.59%
===========================================
Files 136 39 -97
Lines 3576 1004 -2572
Branches 864 273 -591
===========================================
- Hits 2715 656 -2059
+ Misses 861 348 -513
```
| Flag | Coverage Δ | |
|---|---|---|
| frontend-e2e-test | `65.33% <50.00%> (-10.59%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/2573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [web/src/components/Plugin/PluginPage.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbi9QbHVnaW5QYWdlLnRzeA==) | `82.35% <50.00%> (-16.48%)` | :arrow_down: |
| [.../src/pages/User/components/LoginMethodPassword.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Vc2VyL2NvbXBvbmVudHMvTG9naW5NZXRob2RQYXNzd29yZC50c3g=) | `23.07% <0.00%> (-65.39%)` | :arrow_down: |
| [web/src/helpers.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9oZWxwZXJzLnRzeA==) | `37.70% <0.00%> (-42.63%)` | :arrow_down: |
| [web/src/components/RightContent/AvatarDropdown.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1JpZ2h0Q29udGVudC9BdmF0YXJEcm9wZG93bi50c3g=) | `50.00% <0.00%> (-32.15%)` | :arrow_down: |
| [web/src/components/PanelSection/index.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1BhbmVsU2VjdGlvbi9pbmRleC50c3g=) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| [web/src/pages/User/Login.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Vc2VyL0xvZ2luLnRzeA==) | `36.66% <0.00%> (-23.34%)` | :arrow_down: |
| [web/src/pages/Consumer/List.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Db25zdW1lci9MaXN0LnRzeA==) | `69.44% <0.00%> (-22.23%)` | :arrow_down: |
| [...b/src/components/Plugin/UI/referer-restriction.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbi9VSS9yZWZlcmVyLXJlc3RyaWN0aW9uLnRzeA==) | `69.69% <0.00%> (-21.22%)` | :arrow_down: |
| [web/src/components/ActionBar/ActionBar.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL0FjdGlvbkJhci9BY3Rpb25CYXIudHN4) | `69.23% <0.00%> (-15.39%)` | :arrow_down: |
| ... and [106 more](https://codecov.io/gh/apache/apisix-dashboard/pull/2573/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Morakes commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
Morakes commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r939897067
##########
web/src/components/Plugin/PluginPage.tsx:
##########
@@ -280,11 +284,10 @@ const PluginPage: React.FC<Props> = ({
const newPlugins = {
...initialData,
[name]: { ...monacoData, disable: !formData.disable },
+ ...(shouldDelete ? { [name]: null } : {}),
};
- if (shouldDelete === true) {
- newPlugins[name] = null;
- }
- onChange(newPlugins, form.getFieldValue('plugin_config_id'));
+ const handleType = shouldDelete ? 'delete' : 'edit';
+ onChange(newPlugins, handleType, form.getFieldValue('plugin_config_id'));
Review Comment:
Well, based on your hints, I think I've found the root cause，thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#issuecomment-1207708209
also cc @SkyeYoung @Baoyuantop PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Morakes commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
Morakes commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r939756114
##########
web/src/components/Plugin/PluginPage.tsx:
##########
@@ -277,12 +277,12 @@ const PluginPage: React.FC<Props> = ({
setName(NEVER_EXIST_PLUGIN_FLAG);
}}
onChange={({ monacoData, formData, shouldDelete }) => {
- let newPlugins = {
+ const newPlugins = {
...initialData,
[name]: { ...monacoData, disable: !formData.disable },
};
if (shouldDelete === true) {
- newPlugins = omit(newPlugins, name);
+ newPlugins[name] = null;
Review Comment:
OK ,I will modify it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi merged pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
guoqqqi merged PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Morakes commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
Morakes commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r944086403
##########
web/cypress/integration/plugin/create-delete-in-drawer-plugin.spec.js:
##########
@@ -107,6 +109,48 @@ context('Delete Plugin List with the Drawer', () => {
cy.contains('button', 'Confirm').click({
force: true,
});
+ cy.get(selector.notification).should('contain', 'Delete Plugin Successfully');
+ cy.get(selector.notificationCloseIcon).click({ multiple: true });
+ cy.get(selector.empty).should('be.visible');
+ });
+
+ it('should delete the plugin with the drawer in the list of plugins', function () {
+ cy.visit('/plugin/list');
+ cy.get(selector.refresh).click();
+ cy.contains('Enable').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+ cy.get(selector.drawer)
+ .should('be.visible')
+ .within(() => {
+ cy.get(selector.disabledSwitcher).click();
+ cy.get(selector.checkedSwitcher).should('exist');
+ });
+ cy.contains('button', 'Submit').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+
+ cy.contains('button', 'Delete').click({
+ force: true,
+ });
+ cy.contains('button', 'Confirm').click({
+ force: true,
+ });
+ cy.get(selector.notification).should('contain', 'Delete Plugin Successfully');
+ cy.get(selector.notificationCloseIcon).click({ multiple: true });
Review Comment:
OK, I'll improve it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Morakes commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
Morakes commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r939782688
##########
web/cypress/integration/plugin/create-delete-in-drawer-plugin.spec.js:
##########
@@ -109,4 +109,42 @@ context('Delete Plugin List with the Drawer', () => {
});
cy.get(selector.empty).should('be.visible');
});
+
+ it('should delete the plugin with the drawer in the list of plugins', function () {
+ cy.visit('/plugin/list');
+ cy.get(selector.refresh).click();
+ cy.contains('Enable').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+ cy.get(selector.drawer)
+ .should('be.visible')
+ .within(() => {
+ cy.get(selector.disabledSwitcher).click();
+ cy.get(selector.checkedSwitcher).should('exist');
+ });
+ cy.contains('button', 'Submit').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
Review Comment:
If you don't use force it will make the test unstable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#issuecomment-1211464348
also cc @SkyeYoung PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Baoyuantop commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
Baoyuantop commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r944051579
##########
web/src/pages/Plugin/PluginMarket.tsx:
##########
@@ -48,14 +48,18 @@ const PluginMarket: React.FC = () => {
initialData={initialData}
type="global"
schemaType="route"
- onChange={(pluginsData) => {
+ onChange={(pluginsData, pluginId, handleType) => {
createOrUpdate({
plugins: {
- ...initialData,
...pluginsData,
},
Review Comment:
If there is only one left, there is no need to expand the operation.
##########
web/cypress/integration/plugin/create-delete-in-drawer-plugin.spec.js:
##########
@@ -107,6 +109,48 @@ context('Delete Plugin List with the Drawer', () => {
cy.contains('button', 'Confirm').click({
force: true,
});
+ cy.get(selector.notification).should('contain', 'Delete Plugin Successfully');
+ cy.get(selector.notificationCloseIcon).click({ multiple: true });
+ cy.get(selector.empty).should('be.visible');
+ });
+
+ it('should delete the plugin with the drawer in the list of plugins', function () {
+ cy.visit('/plugin/list');
+ cy.get(selector.refresh).click();
+ cy.contains('Enable').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+ cy.get(selector.drawer)
+ .should('be.visible')
+ .within(() => {
+ cy.get(selector.disabledSwitcher).click();
+ cy.get(selector.checkedSwitcher).should('exist');
+ });
+ cy.contains('button', 'Submit').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+
+ cy.contains('button', 'Delete').click({
+ force: true,
+ });
+ cy.contains('button', 'Confirm').click({
+ force: true,
+ });
+ cy.get(selector.notification).should('contain', 'Delete Plugin Successfully');
+ cy.get(selector.notificationCloseIcon).click({ multiple: true });
Review Comment:
After that, we would better check that the plugin button is turned to `Enable` instead of `Edit`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] LiteSun commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
LiteSun commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r943045805
##########
web/cypress/integration/plugin/create-delete-in-drawer-plugin.spec.js:
##########
@@ -109,4 +109,42 @@ context('Delete Plugin List with the Drawer', () => {
});
cy.get(selector.empty).should('be.visible');
});
+
+ it('should delete the plugin with the drawer in the list of plugins', function () {
+ cy.visit('/plugin/list');
+ cy.get(selector.refresh).click();
+ cy.contains('Enable').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+ cy.get(selector.drawer)
+ .should('be.visible')
+ .within(() => {
+ cy.get(selector.disabledSwitcher).click();
+ cy.get(selector.checkedSwitcher).should('exist');
+ });
+ cy.contains('button', 'Submit').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
Review Comment:
Perhaps it would be better to use a more precise selector?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on a diff in pull request #2573: fix: drawer components delete plugin not working
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on code in PR #2573:
URL: https://github.com/apache/apisix-dashboard/pull/2573#discussion_r939677456
##########
web/cypress/integration/plugin/create-delete-in-drawer-plugin.spec.js:
##########
@@ -109,4 +109,42 @@ context('Delete Plugin List with the Drawer', () => {
});
cy.get(selector.empty).should('be.visible');
});
+
+ it('should delete the plugin with the drawer in the list of plugins', function () {
+ cy.visit('/plugin/list');
+ cy.get(selector.refresh).click();
+ cy.contains('Enable').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
+ });
+ });
+ cy.get(selector.drawer)
+ .should('be.visible')
+ .within(() => {
+ cy.get(selector.disabledSwitcher).click();
+ cy.get(selector.checkedSwitcher).should('exist');
+ });
+ cy.contains('button', 'Submit').click();
+
+ cy.contains(data.basicAuthPlugin)
+ .parents(selector.pluginCardBordered)
+ .within(() => {
+ cy.get('button').click({
+ force: true,
Review Comment:
It is not usually advisable to use so many `force` attributes, as it may cause unsuccessful clicks, and we should use some assertions to ensure that the state of the button is normal.
##########
web/src/components/Plugin/PluginPage.tsx:
##########
@@ -277,12 +277,12 @@ const PluginPage: React.FC<Props> = ({
setName(NEVER_EXIST_PLUGIN_FLAG);
}}
onChange={({ monacoData, formData, shouldDelete }) => {
- let newPlugins = {
+ const newPlugins = {
...initialData,
[name]: { ...monacoData, disable: !formData.disable },
};
if (shouldDelete === true) {
- newPlugins = omit(newPlugins, name);
+ newPlugins[name] = null;
Review Comment:
This method is not very good, we do not usually modify the value of an object's properties directly.
An extended question, we should normally have an indication of successful deletion when we delete data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org