Search Filter Improvements #1401

Sherlouk · 2021-11-26T12:39:58Z

Description

This pull request aims to add some finishing touches to the search filter capability which has been enabled in staging for quite a while now (sorry).

Specifically, this makes two changes:

Search filters are visually separated from the search term as part of the search results page
Search filters are applied only to packages, not to keywords/authors allowing for these results to show side-by-side

There will be further improvements to search filters including the ability to filter by keyword and more, but this can could come after it's production-ised and certainly as a separate PR.

Association

Closes #1228
Closes #318
Related to #1396
Conflicts with #1390

This was originally disabled due to the UI work being incomplete. UI improvements have since happened which means we can turn this back on. This means users can search, with filters, and still have author/keyword suggestions based on the unfiltered search terms. `test stars:>1000` will show test packages with at least 1000 stars but will also show the "test" keyword.

This adds two new values to the search response. A "term" which is a subset of the query minus the filters, and "filters" which is currently an array of strings but this is likely to change in a future commit (just an empty array throughout). The search term is different to the search query as with a query of "test stars:>500 framework" the term is simply "test framework" and the filters is ["stars:>500"]. This is because in the UI for the search results, we want to show the term separate to the filters. However, we still need the raw queries for some parts of the UI (for example the user-input search box).

Filters are now shown as part of the search results page under the packages header.

Sherlouk · 2021-11-26T13:03:39Z

This is what this branch currently looks like. Dave has already said he hates everything I do visually and so will come through and clean up the UI anyway ❤️

daveverwer · 2021-11-26T13:34:10Z

Dave has already said he hates everything I do visually and so will come through and clean up the UI anyway ❤️

I don't remember saying that! 😅 But I think this is 95% there already. I really like it. 👍

Sherlouk · 2021-11-26T13:36:55Z

Just in case somebody reads this and thinks that Dave is like supremely evil, it was a joke 😅 Dave and I already agreed he was going to go through and do the UI on Discord ahead of me picking this up! (CSS is not my forte)

daveverwer · 2021-11-26T20:08:04Z

This is looking really, really good. A few things I found while giving it a test tonight:

No filter descriptions when no package results are displayed

So I had coded the CSS for the keyword results never expecting a situation where there would be a keyword result without any package results, which breaks the layout a bit! However, the bigger problem here is that without package results we're not seeing the filter description.

I wonder if we should make it so this page always displays package results, or maybe just always display them if filters have been applied. Looking at the design for a search that matches nothing, it's OK, but it'd also be fine if that text were below the "Matching packages" heading.

More to follow...

daveverwer · 2021-11-26T20:49:00Z

Bug with `last_commit` filters?

On staging, searching for last_commit:>2020-01-01 returns plenty of results, but on this branch, it returns nothing.

Also the same result for anything where there's no search term. for example stars:>500 returns nothing where test stars:>500 returns plenty.

If this is unrelated to this code, we can log this as a separate issue. We should also introduce a test case to check this.

daveverwer · 2021-11-26T21:04:16Z

Should we give the fields more descriptive names in the filter description?

Rather than:

What do you think about:

I'm not sure about this, but last_commit is not ideal.

It might just be that we need:

daveverwer · 2021-11-26T21:08:10Z

Pushed up a few small CSS tweaks but this is in great shape on the front end. 🎉 Thanks James!

Sherlouk · 2021-11-26T21:25:08Z

Appreciate the support Dave!

There's definitely a known (expected) bug where it will return zero results if you only include a filter.

That's just down to how the search mechanism works at the moment because it needs something to search and then just uses the filter to... well... filter down those options.

Probably need to add something in there which lets it search all packages if the term is empty and filters are present. I'd maybe think tackle that separately though?

As for the names as shown in the view, that's easily changeable. The createViewModel function just uses Self.key at the moment but there's zero reason that couldn't be a more human-readable string. Do you want to pick that up under this?

I was also thinking whether the date one wants to use a formatted value in the view model? So instead of "Last Commit is greater than 2020-01-01" it would be more like "Last Commit is greater than 1st Jan 2020". Again, something that could be changed purely in the createViewModel function (and why I made it a separate model).

daveverwer · 2021-11-26T21:30:03Z

There's definitely a known (expected) bug where it will return zero results if you only include a filter.

That's just down to how the search mechanism works at the moment because it needs something to search and then just uses the filter to... well... filter down those options.

Probably need to add something in there which lets it search all packages if the term is empty and filters are present. I'd maybe think tackle that separately though?

One reason I put this in here rather than as a separate issue is that searches without terms but with filters work on staging:

https://staging.swiftpackageindex.com/search?query=stars%3A%3E30000
http://localhost:8080/search?query=stars%3A%3E30000

daveverwer · 2021-11-26T21:32:31Z

As for the names as shown in the view, that's easily changeable. The createViewModel function just uses Self.key at the moment but there's zero reason that couldn't be a more human-readable string. Do you want to pick that up under this?

Yea, let's do that. I think I prefer option 3 out of all of the screenshots. How about you?

Sherlouk · 2021-11-26T21:32:33Z

I wonder if the fact we do a union now for keyword/author search is killing the entire search.

That's the only change I can imagine causing that.

I'll try reverting that in the morning and test.

(First commit in this PR if you want to try)

daveverwer · 2021-11-26T21:33:08Z

I wonder if the fact we do a union now for keyword/author search is killing the entire search.

That's the only change I can imagine causing that.

I'll try reverting that in the morning and test.

Let's write a test for it too to stop future regressions.

daveverwer · 2021-11-26T21:33:39Z

I was also thinking whether the date one wants to use a formatted value in the view model? So instead of "Last Commit is greater than 2020-01-01" it would be more like "Last Commit is greater than 1st Jan 2020". Again, something that could be changed purely in the createViewModel function (and why I made it a separate model).

Great idea 👍

Dave identified an issue where results would only be returned if there was at least one non-filter term in the search query. This was different to staging. I have fixed this and added a test which enforces it.

Sherlouk · 2021-11-27T11:11:44Z

I've done a number of updates and this is where I'm at with it.

This helps to make it read a little better to the user.

Small UX improvement as it reads a bit weirdly.

Moves "no packages found" error down to under the matching packages header. This is a little more resilient to situations where there are keyword/author results (ignore filters) but no packages due to the filter.

Sherlouk · 2021-11-27T11:38:30Z

Think I've now responded to all of the feedback/bugs. Should be good for a review/test

Sherlouk · 2021-11-27T11:55:30Z

I know I shouldn't be proud of this... but every commit passed on CI and that's a good feeling

Little easier for the user to write.

Sherlouk · 2021-11-27T14:39:54Z

As per chat with Dave,appStoreCompatible has now been renamed to compatible.

Sherlouk · 2021-11-27T14:48:16Z

Current

last_commit 
stars
license

Coming Soon

last_activity

Future Ideas

last_release (last_release:>2021-08-08)
dependencies (dependencies:<5)
dependants   (dependants:>100)
compatible   (compatible:>swift-5 compatible:linux)
libraries    (libraries:>0)
executables  (executables:>0)
keyword      (keyword:test)
author       (author:apple author:sherlouk)

daveverwer

Tested locally and everything looks perfect!

I did just notice one tiny code clean up in my final review with the number formatter.

Sources/App/Core/SearchFilter.swift

daveverwer · 2021-11-27T18:07:30Z

Sources/App/Core/SearchFilter.swift

+        formatter.dateFormat = "d MMM yyyy"
+        formatter.locale = Locale(identifier: "en_US_POSIX")
+        formatter.timeZone = TimeZone(secondsFromGMT: 0)
+


We don't have one for dates but I wonder if we should pull this out so our formatters are a little more organised? Equally, I could do this as part of expanding the filters to include last_active. Noted for inclusion in #1390.

Probably makes sense to have a generic "search filter date formatter" for all filters which have a date input. My typical coding brain would say refactor that once there's a need, so in 1390, though. I'm happy to sort out that rebase+cleanup if need be once this gets merged through.

Sources/App/Views/Search/SearchShow+View.swift

…tPackageIndex-Server into searchfilterstuff

daveverwer · 2021-11-27T18:30:17Z

This is an amazing feature, @Sherlouk. Thank you so much for all of your efforts with it.

❤️

Sherlouk · 2021-11-27T18:31:33Z

Sherlouk added 4 commits November 26, 2021 12:31

Create search filter view model and pass through to search results view

5d98349

Filters are now shown as part of the search results page under the packages header.

Basic styling improvements

4ef56f0

Sherlouk added 2 commits November 26, 2021 13:14

Consider prefixing with ampersand

6af7e77

Add some new tests covering search filters

116b2a9

Sherlouk changed the title ~~[WIP] Search Filter Improvements~~ Search Filter Improvements Nov 26, 2021

CSS tweaks.

4bbe69c

Snapshots.

5753ae3

Fix bug where no results returned with only filters

44dd42a

Dave identified an issue where results would only be returned if there was at least one non-filter term in the search query. This was different to staging. I have fixed this and added a test which enforces it.

Sherlouk added 3 commits November 27, 2021 11:15

Format key/values in filter view model

8b441cf

This helps to make it read a little better to the user.

Hide 'results for' header when no term present

ccdadc8

Small UX improvement as it reads a bit weirdly.

Restructure search empty views

8264f4b

Moves "no packages found" error down to under the matching packages header. This is a little more resilient to situations where there are keyword/author results (ignore filters) but no packages due to the filter.

Sherlouk marked this pull request as ready for review November 27, 2021 11:38

daveverwer and others added 5 commits November 27, 2021 12:51

Fix optional warning.

daef91f

Wording tweaks.

3efb13b

Fixed up tests after my wording tweaks.

da8cecb

Snapshots.

033ec04

Rename appStoreCompatible to compatible

cfe48ad

Little easier for the user to write.

daveverwer mentioned this pull request Nov 27, 2021

Add a last activity filter #1390

Closed

daveverwer approved these changes Nov 27, 2021

View reviewed changes

Sherlouk and others added 3 commits November 27, 2021 18:22

Use shared stars formatter

594e6ef

Deal with empty search terms, only filters.

007bdeb

Merge branch 'searchfilterstuff' of github.com:SwiftPackageIndex/Swif…

8dc92a6

…tPackageIndex-Server into searchfilterstuff

daveverwer enabled auto-merge November 27, 2021 18:29

daveverwer merged commit 7fea3c3 into main Nov 27, 2021

daveverwer deleted the searchfilterstuff branch November 27, 2021 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search Filter Improvements #1401

Search Filter Improvements #1401

Sherlouk commented Nov 26, 2021 •

edited

Sherlouk commented Nov 26, 2021

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 26, 2021

daveverwer commented Nov 26, 2021

daveverwer commented Nov 26, 2021 •

edited

daveverwer commented Nov 26, 2021

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 26, 2021

daveverwer commented Nov 26, 2021 •

edited

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 26, 2021 •

edited

daveverwer commented Nov 26, 2021

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021 •

edited

daveverwer left a comment

daveverwer Nov 27, 2021

Sherlouk Nov 27, 2021

daveverwer commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Search Filter Improvements #1401

Search Filter Improvements #1401

Conversation

Sherlouk commented Nov 26, 2021 • edited

Description

Association

Sherlouk commented Nov 26, 2021

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 26, 2021

daveverwer commented Nov 26, 2021

No filter descriptions when no package results are displayed

daveverwer commented Nov 26, 2021 • edited

Bug with last_commit filters?

daveverwer commented Nov 26, 2021

Should we give the fields more descriptive names in the filter description?

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 26, 2021

daveverwer commented Nov 26, 2021 • edited

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 26, 2021 • edited

daveverwer commented Nov 26, 2021

daveverwer commented Nov 26, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 27, 2021 • edited

daveverwer left a comment

Choose a reason for hiding this comment

daveverwer Nov 27, 2021

Choose a reason for hiding this comment

Sherlouk Nov 27, 2021

Choose a reason for hiding this comment

daveverwer commented Nov 27, 2021

Sherlouk commented Nov 27, 2021

Sherlouk commented Nov 26, 2021 •

edited

daveverwer commented Nov 26, 2021 •

edited

Bug with `last_commit` filters?

daveverwer commented Nov 26, 2021 •

edited

Sherlouk commented Nov 26, 2021 •

edited

Sherlouk commented Nov 27, 2021 •

edited