Skip to content
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

Disable apply button at initial state and move agents to their original position #3605

Merged
merged 15 commits into from Nov 5, 2021

Conversation

sortiz1191
Copy link
Contributor

@sortiz1191 sortiz1191 commented Sep 15, 2021

Hi team!

When managing agents into a group generates an error, all the agents move to their original positions.
Also, the Apply changes button is disabled when all the agents are in their original position.

To test it:

  1. Go to Management/Groups
  • Given the browser is in wazuh app with more than 1 user available
  • When the user navigates to Management/Groups
  • Then the Groups section will be shown
  1. Create a Group
  • Given the Group section
  • When the user click in add new group
  • And the user fill the name with "test"
  • And the user click on "Save new Group"
  • Then the group will be displayed in Groups table
  1. Enter in Manage Agents in Groups section
  • Given the group section with the group "test" created
  • When the user click on test
  • And the user click on "Manage agents" in the Test group section
  • Then the agents will be displayed in Available agents and current agents in the group will be empty
  1. Try to move agents in Manage agents of group test
  • Given the section to move agent from some groups
  • When the user can move all agents to the right or left back, save them and move them again
  • Then the button to save will be enabled if the user move some of the users and there isn't any errors (except when the user will try to move an inactive or never connected agent, that will persist in the left side)

Solves: #3332

@sortiz1191 sortiz1191 requested a review from a team September 15, 2021 11:20
@sortiz1191 sortiz1191 self-assigned this Sep 15, 2021
Copy link
Member

@asteriscos asteriscos left a comment

Choose a reason for hiding this comment

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

Test: ❌

When one of the agents is added and the rest is not, it doesn't end in the right group column. Also, the See full error doesn't do anything. In addition, the number of added or removed items is not reset after the operation.

Peek 2021-09-15 13-42

Copy link
Member

@mpRegalado mpRegalado left a comment

Choose a reason for hiding this comment

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

The code performs the function required. Upon encountering an error, it reverts all the changes to its original state.
However, the way that it does so causes further errors to be logged into the console.

Examining the code in wzRequest, it rejects the promise if the response contains failed_ids, never updating the failedIds array and causing the code to attempt and remove agents that do not belong in the group, causing the error.

from.splice(idx, 1);
item.type = !item.type ? type : '';
to.push(item)
moveItem = (item, from, to, type) => {
Copy link
Member

Choose a reason for hiding this comment

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

This functions mutates the state. It doesn't seem to have been changed in this PR but it should be addressed in another issue

@asteriscos
Copy link
Member

asteriscos commented Oct 18, 2021

Test: ❌
There's an odd behavior when adding multiple agents and some of them throw an error after saving the changes.
The agents that were successfully added are not shown in the group and the changes can be seen after reloading the page or switching views.

The expected result would be to show the successfully added agents in the right column and the ones that failed on the left.

Peek 2021-10-18 18-50

Copy link
Member

@mpRegalado mpRegalado left a comment

Choose a reason for hiding this comment

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

Error in console persists

Copy link
Contributor

@CPAlejandro CPAlejandro left a comment

Choose a reason for hiding this comment

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

I have just seen this bug, when you put the agent in a group, you move it back, saving all times and then try to enter the group another time, the button is disabled.

Copy link
Member

@asteriscos asteriscos left a comment

Choose a reason for hiding this comment

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

Test: ❌

There seems to be an issue with the state of added and removed agents.
After adding or removing one agent, its state cannot be changed again.

Peek 2021-10-28 13-46

Peek 2021-10-28 13-50

Copy link
Member

@asteriscos asteriscos left a comment

Choose a reason for hiding this comment

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

Test: ✔️
CR: ✔️
Possible minor improvements

<option
key={index}
className={
item.type === 'a'
Copy link
Member

Choose a reason for hiding this comment

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

Possible small Improvement: You could map classNames with the item.type so the className prop of the option is easier to read.
like className={ typeClasses[item.type] }

<option
key={index}
className={
item.type === 'a'
Copy link
Member

Choose a reason for hiding this comment

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

Possible small Improvement: You could map classNames with the item.type so the className prop of the option is easier to read.
like className={ typeClasses[item.type] }

Copy link
Contributor

@CPAlejandro CPAlejandro left a comment

Choose a reason for hiding this comment

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

CR and Testing: LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Bug issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants