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

Extract price properties and index as double #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bjarnef
Copy link

@bjarnef bjarnef commented Sep 14, 2023

While playing with the upcoming facets in Examine v4, I found we can't use the existing indexed "price" field as facet field.
Shazwazza/Examine#310

https://github.com/Shazwazza/Examine/blob/release/4.0/docs/v2/articles/configuration.md

With facets the index configuration would look something like this:

public void Configure(string name, LuceneDirectoryIndexOptions options)
{
    switch (name)
    {
        case Constants.UmbracoIndexes.ExternalIndexName:

            var priceFields = new List<string>
            {
                "price_GBP"
            };

            // Create a config
            var facetsConfig = new FacetsConfig();

            foreach (var field in priceFields)
            {
                options.FieldDefinitions.TryAdd(new FieldDefinition(field, FieldDefinitionTypes.FacetDouble));
                facetsConfig.SetIndexFieldName(field, $"facet_{field}");
            }

            options.FacetsConfig = facetsConfig;
            //options.UseTaxonomyIndex = true;

            break;
    }
}
var searcher = index.Searcher;
var query = searcher.CreateQuery().NativeQuery(q);

var results = query.OrderBy(new SortableField("name", SortType.String))
    .WithFacets(facets => facets
        .FacetDoubleRange("price_GBP", new DoubleRange[] {
            new DoubleRange("0-10", 0, true, 10, true),
            new DoubleRange("11-20", 11, true, 20, true),
            new DoubleRange("20-30", 21, true, 30, true)
        })) // Get facets of the price field
    .Execute(QueryOptions.SkipTake(pageSize * (page - 1), pageSize));

var facets = results.GetFacets(); 

@mattbrailsford
Copy link
Contributor

Looks good. I just wish ConfigureIndexOptions supported DI such that you could load the currencies to dynamically generate the keys, rather than them being hard coded. It just makes it something that isn't obvious to update. Maybe if they could be put into app settings or something, I'm not sure. Just some way of surfacing the support currencies 🤔

@bjarnef
Copy link
Author

bjarnef commented Sep 14, 2023

@mattbrailsford do you mean something like this?

image

The documentation shows an example where ILoggerFactory is injected.. I tried with ICurrencyService but doesn't seem to work.
https://shazwazza.github.io/Examine/articles/configuration.html

@bjarnef
Copy link
Author

bjarnef commented Sep 14, 2023

I tried var currencyService = StaticServiceProvider.Instance.GetRequiredService<ICurrencyService>(); but didn't seem to work either.

Not sure if it is related to any specific implementation in Umbraco, but it seems it may be possible to use DI of an service?
https://andrewlock.net/configuring-named-options-using-iconfigurenamedoptions-and-configureall/#using-injected-services-when-configuring-all-options-instances

https://andrewlock.net/simplifying-dependency-injection-for-iconfigureoptions-with-the-configureoptions-helper/

@mattbrailsford
Copy link
Contributor

Yea, I've hit this issue in another solution. The problem is these things are registered at registration time and so the DI container hasn't been built yet. The logger works because it's explicitly constructed that early and is available to be passed in. It's not actually being resolved by DI.

@bjarnef
Copy link
Author

bjarnef commented Sep 14, 2023

Ahh okay, I see.
Maybe configuring price fields in appsettings.json as you mentioned, will work for now.

image

I noticed the demostore project has this in appsettings.json:
https://github.com/umbraco/Umbraco.Commerce.DemoStore/blob/main/src/Umbraco.Commerce.DemoStore.Web/appsettings.json#L43

but I only see HttpOnly property on the UmbracoCommerceCookieSettings class.. am I missing something?

namespace Umbraco.Commerce.Core.Configuration.Models
{
    public class UmbracoCommerceCookieSettings
    {
        public bool HttpOnly { get; set; }
    }
}

@bjarnef
Copy link
Author

bjarnef commented Sep 14, 2023

@mattbrailsford if you want to try out facets on demostore (maybe a new branch?) there a local NuGet package for Examine v4: Shazwazza/Examine#310 (comment) ... but soon there will be a public beta (pre-release) NuGet package.

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.

2 participants