ChrisASearles / CompLinks

0 stars 0 forks source link

Error in Search #143

Closed ChrisASearles closed 7 years ago

ChrisASearles commented 7 years ago

Searches that return no results are throwing an unhandled exception in AdvertiserDAL.cs on line 200. advertiser.AdvertiserCategories is empty and the .First() is failing out.

To reproduce, just run a search for 'm'.

cgladue commented 7 years ago

@Dwells7583 @ChrisASearles

Can you please put a minimum length on the search bar ?

@robgpeak what do you think, 3 characters minimum on the search ?

robgpeak commented 7 years ago

Yeah, I suppose that's ok. Although I was thinking we were going to use the auto fill idea... Meaning, as someone starts typing the name of a store, it would start filling in.

cgladue commented 7 years ago

We can do that, but there should still be a minimum # or characters to trigger the auto complete. or if they choose not to select anything that the autocomplete presents and just press enter.

robgpeak commented 7 years ago

Would it really hurt to do it the way ebates does it, where the auto complete starts even at the first letter being typed in, and the results appear instantly in the search bar (as a drop down) itself.

ChrisASearles commented 7 years ago

@robgpeak need to push that back to a future release. @cgladue I'm going to have to push back on the minimum character count on the search. I understand using a minimum in an async auto-fill before we start running searches but it's a rough user experience, especially since there's no elegant way to present any feedback to the user that the number of characters is the reason nothing happens when they hit the enter key. I saw an OOM exception the other day when I ran one, why not just limit the results you're pulling back?

cgladue commented 7 years ago

up to you, but the search will be CONTAINS(term) not = term or startswith term so if i search for m it will pull any advertiser that has an m in their name,

cgladue commented 7 years ago

@robgpeak i have a functional autocomplete already done, just threw it together in 5 mins it will be live soon and just need styling

cgladue commented 7 years ago

@robgpeak try the searchbar now...

robgpeak commented 7 years ago

Maybe instead of overall "startswith", we can do, "any word in Advertiser Name that startswith"

You'll see when I type "ala" Alamo, Alamy, and some others that start with ala show up, but so does "University of Alabama Gift Shop...

Then I would be able to go in and change some names of advertisers, like Bestofvegas.com to Best of Vegas, so if someone typed Vegas, it would show up in the results when they got to veg

screen shot 2017-06-01 at 11 12 00 pm
robgpeak commented 7 years ago

I think what you have now is a step in the right direction, but with just "contains", it yields too many irrelevant results.

cgladue commented 7 years ago

ok, now it should be "StartsWith" or "Contains Word Starting With" Logic

cgladue commented 7 years ago

image

robgpeak commented 7 years ago

Works great @cgladue

A couple of issues:

  1. Unfortunately the same functionality does NOT work when the screen size is reduced, i.e. mobile.

  2. The same functionality does NOT work AFTER you type a word, or part of one and hit enter. Meaning, if you are on the https://shop.complinks.co/stores/search page after you've just searched for something, the "StartsWith" or "Contains Word Starting With" Logic does not work. Try it and you'll see what I mean.

  3. If someone types just one letter, AND PRESSES ENTER, it seems like the logic is different, and it is actually using "Contains" Logic, and is returning every that contains that letter. That's obviously no good. I think the fix there is "If the search only contains one letter, AND someone hits ENTER without manually selecting a particular store in the dropdown search bar result, only display those stores that "StartsWith" or "Contains Word Starting With" Logic"

  4. This next one is about the "You May Also Like" section, but I am going to include it here because it relates to the search results. It appears that these store boxes in this section are being selected by taking the stores in the search results, looking at their first (alphabetically) category, and displaying all the other stores in that category.

While that sound good in theory, it does not work in practicality terms. I know we want to do something, and it is a logical choice, but again, in practice it does not always return the desired result. Having said that, and for now, at least in this phase, I think we need to think of the You May Also Like section as exactly that, not Similar, just that they might like it...

In that instance, I think the fix would be, or at least the best one I can think of at the moment... "If the search results yield MULTIPLE store boxes that are in DIFFERENT categories, only show stores in the 'You May Also Like' section from the FIRST category, of the FIRST store in the search results."

So as an example, you type 'group' in the search bar and hit enter, you'll see 4 results in the search, and a shitload in the You May Also Like section. Based on this logic above, the You May Also Like section would only show stores that are in the 'Home, Garden & Tools' category instead of all the ones it shows now.

Again, not the optimal solution, but for the purposes of putting a bow on the search and You May Also Like section functionality, let's try it.

ChrisASearles commented 7 years ago

Guys, we need to shelve this for now. This is going to cause a number of unnecessary delays in our initial release and needs to be handled after we go live with a beta. @cgladue we need to disable this functionality for now. I'm going to add this as a separate issue that we can prioritize and work on once we've released something.

cgladue commented 7 years ago

Ill let @robgpeak make that decision, its not up to us.

robgpeak commented 7 years ago

Chad, actually, this was the reason (one of them) that we brought Chris on board, was to make sure we are keeping all my ideas and thoughts in line. If he says at the moment there are more important things that we need to focus on for the launch let's do that.