OpenConext / Stepup-Project

Managing issues for Stepup-* projects
0 stars 0 forks source link

Spurious/mislabeled search filters in search-ra-candidate screen #294

Closed phavekes closed 2 days ago

phavekes commented 2 days ago

This issue is imported from pivotal - Originaly created at Feb 27, 2020 by Pieter van der Meulen

In the RA Management, add RA(A) search screen (https://<ra>/management/search-ra-candidate) the search filters do not match the columns

The Institution search filter is always empty. This filter should filter the Institution column The RA Institution search filter is operational and filters the Institution column

So, to fix: Remove the Institution filter and rename the RA Institution filter to Institution

Steps to reproduce:

  1. Login to the RA with (S)RAA role
  2. Click "RA Management"
  3. Click "+ Add RA(A)"
phavekes commented 2 days ago

Screenshots: (Pieter van der Meulen - Feb 27, 2020)

phavekes commented 2 days ago

The RA institution filter does indeed filter the results on the RA Institution of the ra-candidate projection.

What I\'d like to do is fix the middleware to let it return institution filter values. And if you still want to, remove the RA insititution filter from the UI. I think that will be more intuitive from a users point of view? Do you agree @pmeulen ? Or shall I simply follow the story description.

MW PR proposal: https://github.com/OpenConext/Stepup-Middleware/pull/297 (Michiel Kodde - Feb 27, 2020)

phavekes commented 2 days ago

I noticed that the filter list includes all institutions to which the RAA has rights, and not just those for which there are RA candidates available, which is how the filter works in other places. Is that what you propose? (Pieter van der Meulen - Feb 27, 2020)

phavekes commented 2 days ago

It should be displaying the institutions of the ra-candidates. For example my personal ra_candidates projection does not have institution-e ra-candidates. So that institution is not listed in the selectbox. (Michiel Kodde - Feb 27, 2020)

phavekes commented 2 days ago
@michielkodde Agreed, that would be fully correct and consistent with how filters behave thought the RA. (Pieter van der Meulen - Feb 27, 2020)
phavekes commented 2 days ago

Remaining descision: should we display both the RA- and institution filters? Or only the Institution filter selectbox? (Michiel Kodde - Feb 27, 2020)

phavekes commented 2 days ago

Discussed IRL: the RA filter is to be removed. The fixed (MW PR) institution select list is to remain on the form. (Michiel Kodde - Feb 27, 2020)

phavekes commented 2 days ago

These two releases should fix the bug https://github.com/OpenConext/Stepup-Middleware/releases/tag/3.1.4 π https://github.com/OpenConext/Stepup-RA/releases/tag/3.1.1 (Michiel Kodde - Feb 27, 2020)

phavekes commented 2 days ago

Tested in RA 3.1.2

OK: Only "Institution" search filter is present FAIL: No options "Institution" search filter. (See screenshot)

This is for an RAA with rights in two institutions: Explicit: role in institution -a.nl RAA in institutioj-f.nl though use_raa

As SRAA then filter is filled correctly

There are candidate RA(A)s for both institutions in the list. Tested on TEST2. (Pieter van der Meulen - Mar 5, 2020)

phavekes commented 2 days ago
@mkodde some Behat test did rely on the filtering of actor institution and they also need some TLC. Could you also update those tests accordingly? Maybe just hiding the element would be sufficient? (bstrooband - Mar 6, 2020)
phavekes commented 2 days ago
Update. An new version of the MW needs to be deployed as well (3.1.4). This adds options to the "Institution" search filter.

A new bug is introduced however: RA candidate users from institutions for which the user could be RAA (through select_raa) are shown and the institution is included in the search filter.

@bstrooband Do you want me to make a new issue for this, of do you want to continue to use this one? (Pieter van der Meulen - Mar 6, 2020)

phavekes commented 2 days ago
@michielkodde I\'ll have a fix almost ready for the tests, so I\'ll hang a PR on this issue when it\'s ready.

@pmeulen I wouldlike to have an issue if possible. Then it will be more clear for future references and serve as better documentation` in the end. (bstrooband - Mar 6, 2020)

phavekes commented 2 days ago

Test on test2. Looks OK now. (Pieter van der Meulen - Mar 17, 2020)