kevbite / CompaniesHouse.NET

A simple .NET client wrapper for CompaniesHouse API
MIT License
37 stars 44 forks source link

Issue: SearchCompanyAsync method does not encorporate restrictions query parameter #203

Closed arucodes closed 12 months ago

arucodes commented 1 year ago

Hi Kev,

I was working on consuming this SearchCompanyAsync method for searching companies, however I needed to search for companies that are not having dissolved status, and the companiesHouse provides a query parameter for this as "restrictions". Which helps enhance the search.

Reference:

Without using this parameter suppose I developed an API to consume this method and searched for companies ( using SearchCompanyAsync ) with items_per_page as 10 and received 2 companies as dissolved, it affects my pagination, now I have to give response back with 8 items while the pagination was having 10 items per page for the UI, it looks bad.

Please check.

arucodes commented 1 year ago

@kevbite also your test cases are not buildable as the .net framework on project CompaniesHouse is 4.5 and on test case project ir is .net 7. When i updated the project's targeting framwork to build test cases. They are already failing. Please check this at your own end, I could currently provide a fix but please merge them on priority

kevbite commented 1 year ago

The tests should still run locally, just ran them on GitHub and I don't think it has access to the key because it's a fork.

However, the code you've changed applies restrictions to all search endpoints when it's only applicable to searching companies, I'd be good if the types matched what the api accepts. Also, it seems that restrictions is also an enumeration that is delimited with a space.

https://developer-specs.company-information.service.gov.uk/companies-house-public-data-api/reference/search/search-companies

kevbite commented 1 year ago

@arucodes do you know where the enumations values are for this query string? I can't find them anywhere on their documentation except for the 2 examples.

gurza commented 1 year ago

@kevbite there is a bug in #204

It should be

if (!string.IsNullOrWhiteSpace(request.Restrictions))
{
    query += "&restrictions=" + request.Restrictions;
}

instead of

if (string.IsNullOrWhiteSpace(request.Restrictions))
{
    query += "&restrictions=" + request.Restrictions;
}

https://github.com/kevbite/CompaniesHouse.NET/pull/204/commits/0c113ac70252555fdc69ab7fe45c721c9b8206cb#diff-d09858bb2ecfeec999c19b1adc9557375b62d6e4000ec3df1358bd8bc5d407d0R30

I fixed this bug and did a minor refactoring of the SearchUriBuilder and SearchCompanyUriBuilder classes. Please take a look at my PR #208

gurza commented 1 year ago

@arucodes please look at https://github.com/kevbite/CompaniesHouse.NET/pull/208/commits/d90978ca22837b882eb8876ff3f75ad31208133d

This is a quick fix to add the restrictions parameter to a company search query.

kevbite commented 12 months ago

Hey, sorry taken a while, but I've merged in these changes, they'll be in the latest nuget package.