Angular: Local Admin Search Filter Groups Port

Bug #1844169 reported by Kyle Huckins
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

We’ll be porting the Search Filter Groups UI from DOJO to Angular. This will effectively be two separate grid UIs, one for Search Filter Group(FM class: asfg), and one for Search Filter Group Entries(FM class: asfge). Clicking on the code field of a Group should bring the user to the grid for that group’s Entries. Clicking on the Label field of the Entry should bring you to the fm-editor, displaying for edit the label, query, and position of each entry.

We should be able to select any amount of Search Filter Groups, Delete any selected groups, and add new groups. The same goes for Search Filter Group Entries.

Grid columns available:

Search Filter Group: #, Code, Owner, Label, Create Date

Search Filter Group Entry: #, Query, Sort Position, ID

Mike Risher (mrisher)
Changed in evergreen:
assignee: nobody → Mike Risher (mrisher)
Revision history for this message
Mike Risher (mrisher) wrote :

This is ready for review. Please note that this work needs to use the new "defaultNewRecord" attribute on the <eg-admin-page> component, which is part of the work done on bug 1840287. (this card: https://bugs.launchpad.net/evergreen/+bug/1840287 ). I'll need to come back to this work and add defaultNewRecord once the work on 1840287 is approved.

Here's a link to the branch:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1844169-search-filter-groups

Bill Erickson (berick)
Changed in evergreen:
milestone: none → 3.5-alpha
status: New → Confirmed
importance: Undecided → Wishlist
Revision history for this message
Mike Risher (mrisher) wrote :

Adjustments were made based on feedback. Now the Search Filter Group Entries are always shown in the order of their "pos" value. The "pos" column no longer has a link for sorting.

The branch has been updated:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1844169-search-filter-groups

Changed in evergreen:
assignee: Mike Risher (mrisher) → nobody
tags: added: pullrequest
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Thanks Mike and Kyle. This is looking good. Just a few issues..

I've pushed a branch which includes sign-off, some IDL additions, and other minor fixes.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1844169-angular-search-filter-groups

The branch is still missing part of the feature, though. When adding or editing search group entries, it's not possible to modify the actual search query. It's only possible to use an existing search query.

See the legacy interface (/eg/staff/admin/local/actor/search_filter_group) and note how editing an entry lets you edit both the query and the label. Under the covers, this add/modifies a row in actor.search_query.

tags: removed: pullrequest
Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Mike Risher (mrisher) wrote :

Thank you for the feedback. I revisited this and addressed the missing functionality. It's ready for another look.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1844169-search-filter-groups

tags: added: pullrequest
Changed in evergreen:
milestone: 3.5-beta → 3.5.0
Changed in evergreen:
milestone: 3.5.0 → 3.5.1
Changed in evergreen:
milestone: 3.5.1 → 3.6-beta
Revision history for this message
Mike Rylander (mrylander) wrote :

It looks like this needs a rebase and minor adjustment. I'm getting the following when building eg2 in master-ish:

    ERROR in src/app/staff/admin/local/search-filter/search-filter-group.component.ts:38:9 - error TS2554: Expected 9 arguments, but got 7.

    38 super(route, idl, org, auth, pcrud, perm, toast);
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

      src/app/staff/share/admin-page/admin-page.component.ts:129:9
        129 private perm: PermService,
                    ~~~~~~~~~~~~~~~~~~~~~~~~~
        An argument for 'perm' was not provided.

Changed in evergreen:
milestone: 3.6-beta → 3.next
Revision history for this message
Mike Risher (mrisher) wrote :

Mike Rylander, I did the rebase and fixed the error you reported plus a few others that came up. It's ready for another look:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1844169-search-filter-groups-v2

Andrea Neiman (aneiman)
Changed in evergreen:
assignee: nobody → Andrea Neiman (aneiman)
Revision history for this message
Andrea Neiman (aneiman) wrote :

Testing this on pattypan and it all looks nice, but I don't seem to be able to get new Search Filter Group Entries to save. I was able to edit and delete existing entries, but creating a new entry gives me a confirmation toast but doesn't actually display the new entry in the grid (even after refresh, and then backing out and coming back into the Search Filter Group Entries page.

I was able to create a new Search Filter Group, but when I attempted to add any entries I got an error "Error! Some fields not filled out!" even though all fields were filled out.

I will note that this is not an interface I am highly familiar with (nor does there appear to be much in the way of docs), but, I was able to edit, delete, and add new entries in a 3.6 system running the old Dojo version of this interface.

Changed in evergreen:
assignee: Andrea Neiman (aneiman) → nobody
Revision history for this message
Andrea Neiman (aneiman) wrote :

Addendum/correction:

In a new search filter group (on pattypan, abn_sfg_test) I AM able to create entries, as long as I don't try to use 0 as a position. If I use 0, I get the "some fields not filled out" error. I was successfully able to create group entries with positions 1, 2, 3, and 4.

The Dojo interface, as well as other interfaces that set position (Carousel Mappings) allow 0 as a position entry so that should probably be preserved here.

Revision history for this message
Mike Risher (mrisher) wrote :

Thank you for catching the error involving position 0. I figured out what went wrong. My code was interpreting a "0" as falsy, making it think that 0 indicated the field wasn't filled out.

I added a little bit of extra logic so that it accepts the value of 0. I've tested out the new code by creating an entry and by editing an entry. In both cases I am able to set position to 0.

It's ready for another look. I amended my commit; it's on the same branch.
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1844169-search-filter-groups-v2

Changed in evergreen:
assignee: nobody → Terran McCanna (tmccanna)
Revision history for this message
Jason Boyer (jboyer) wrote :

Hi Mike, it looks like you'll either need to change newQueryPosition to an int or compare it against the string '0'. Building the latest branch gives me this error:

ERROR in src/app/staff/admin/local/search-filter/query-dialog.component.ts:55:64 - error TS2367: This condition will always return 'true' since the types 'string' and 'number' have no overlap.

55 if (!this.newQueryLabel || (!this.newQueryPosition && (this.newQueryPosition != 0)) || !this.newQueryText) {

tags: added: needsrepatch
removed: pullrequest
Revision history for this message
Kyle Huckins (khuckins) wrote :

As Mike's moved on to other projects, I've rebased and updated the branch with the recommended changes. Went with comparing it against the string '0,' as that was confirmed to as expected on IRC: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/khuckins/lp1844169-search-filter-groups-v2

tags: added: pullrequest
removed: needsrepatch
Revision history for this message
Terran McCanna (tmccanna) wrote :

Thanks Kyle and Jason. The interface looks good and I am able to create and edit a Search Filter Group, and create and edit Search Filter Group entries. My sign off is at:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mccanna/lp1844169-search-filter-groups-signoff

tags: added: signedoff
Changed in evergreen:
assignee: Terran McCanna (tmccanna) → nobody
Revision history for this message
Galen Charlton (gmc) wrote :

I have pushed this to master for inclusion in 3.8, along with some follow-ups:

- make the group entry form validated
- add a navigation link from the group edit page back to the list of groups
- fix lint
- add release notes entries

Thanks, Mike, Kyle, and Terran!

Changed in evergreen:
milestone: 3.next → 3.8-beta
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers