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

Fa206 multi select #360

Merged
merged 46 commits into from
Jan 21, 2022
Merged

Fa206 multi select #360

merged 46 commits into from
Jan 21, 2022

Conversation

egbicker
Copy link
Contributor

No description provided.

@egbicker egbicker marked this pull request as ready for review December 21, 2021 21:13
@egbicker egbicker linked an issue Dec 21, 2021 that may be closed by this pull request
@dorschs57
Copy link
Collaborator

I'm seeing an error NoneType object is not iterable when I multiselect or single select a file from the System Trust Database Admin screen and click Add to Ancillary Trust button.

@dorschs57
Copy link
Collaborator

I'm seeing an error NoneType object is not iterable when I multiselect or single select a file from the System Trust Database Admin screen and click Add to Ancillary Trust button.

I see the same thing on the Ancillary Trust Database Admin screen when I attempt to trust/untrust files.

@egbicker
Copy link
Contributor Author

Fixed the NoneType object is not iterable issue and all the code changes you mentioned

@dorschs57
Copy link
Collaborator

Here are just some general comments on this:

  1. The code in the system and ancillary trust database admin pages should be smart enough that I’m not creating a changeset for a file that doesn’t actually change its trust status. For example right now I can multiselect a mixture of trusted and untrusted files. My options on which button (Trust or Untrust) that is active is dictated by the last selected file. If the last file selected was untrusted, I can untrust the whole set of selected file. This means I’m generating changesets to untrust files that are already untrusted. My suggestion would be to only allow an action to take place if all the selected files have the same status. i.e. if they are all untrusted the Trust button is active, if they are all trusted the Untrust button is active, and if they are mixed both button are disabled. Apply similar logic to the system trust page and the Add to Ancillary Trust button.

  2. Multiselect on the Policy Analysis page is useless at this point. We can only display data for a single selected item, and the only action, Reconciliation, doesn’t allow for multi select. Here my suggestions. Eliminate the multiselect on the users and groups. There is no actions that can be taken on them right now and merging multiple users/groups subject list together seems complicated. I don’t know that we want to do that. We can keep the multiselect on subjects and objects lists for now with the idea of the reconciliation wizard to come down the road. I created issue Policy Analysis Reconciliation Wizard #364 for this.

@dorschs57
Copy link
Collaborator

  1. Multiselect on the Policy Analysis page is useless at this point. We can only display data for a single selected item, and the only action, Reconciliation, doesn’t allow for multi select. Here my suggestions. Eliminate the multiselect on the users and groups. There is no actions that can be taken on them right now and merging multiple users/groups subject list together seems complicated. I don’t know that we want to do that. We can keep the multiselect on subjects and objects lists for now with the idea of the reconciliation wizard to come down the road. I created issue Policy Analysis Reconciliation Wizard Policy Analysis Reconciliation Wizard #364 for this.

After talking with @jw3 I'll amend this a bit. I'd still get rid of multiselect on users and groups. On the subjects and objects for this PR:

  1. If the user single selects and right-clicks, keep as is with the Reconcile option
  2. If the user multi-selects and right-clicks, remove the Reconcile option from the context menu and replace it with an option to Trust or Untrust (depending on the trust status of the selected files). If the user selects Trust/Untrust, show a simple Yes/No dialog to confirm and then generate a changeset for all selected items. This would be more of a "bulk action" compared to a wizard. Once Policy Analysis Reconciliation Wizard #364 is in place we can swap it out for that.

@jw3
Copy link
Member

jw3 commented Jan 4, 2022

Change the text of the button on the System Trust tab to be "Add To Ancillary Trust Database", saves us from picking a singular or plural version of "File"

egbicker and others added 8 commits January 13, 2022 16:32
* lowered the confirm_change_dialog min GTK version to 3.22
* fixed subject_list right click handler so it doesn't unselect when doing a multiselect
@dorschs57
Copy link
Collaborator

I submitted a couple easy fixes from my testing.

  1. I lowered the min GTK version on the confirm dialog to 3.22. This is the version we use everywhere.
  2. I returned True from the right-click handler of the subjectList. This stops the event cascading so when you have multiple files selected and right-click it keeps all the files selected.

Copy link
Collaborator

@dorschs57 dorschs57 left a comment

Choose a reason for hiding this comment

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

I'm seeing some issues with files trusted in the system trust database. I'm testing with the fapolicy-analyzer/tests/data/events0.log file.

  1. Select bin user
  2. Multiselect /usr/bin/find and /usr/bin/gzip
  3. Right-click and I'm presented with the Untrust Files option even though both are trusted in the system trust db. (issue 1)
  4. Select Untrust Files and the confirm dialog says 2 files will be trusted. Trust selected files? even though the option was to Untrust the files. (issue 2)
  5. Click Yes to confirm and it appears to create an empty changeset. (issue3)

Copy link
Collaborator

@dorschs57 dorschs57 left a comment

Choose a reason for hiding this comment

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

👍

@egbicker egbicker merged commit f2ce0de into master Jan 21, 2022
@egbicker egbicker deleted the fa206-multi-select branch January 21, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi-select support
3 participants