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

Search API stopped working #13

Closed
christof-mathies opened this issue Mar 22, 2024 · 9 comments
Closed

Search API stopped working #13

christof-mathies opened this issue Mar 22, 2024 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@christof-mathies
Copy link

christof-mathies commented Mar 22, 2024

A few days ago the search api stopped working. There are a few things that need to be addressed to get it working again:

  1. The client_id when logging in needs to be updated to 30100.
  2. The x-picnic-agent header needs to be updated to 30100;1.15.232-15154.
  3. The url has changed to /pages/search-page-results?search_term=.... The response is now json data that contains page layout and analytics information but the search results can be found by looking for selling_unit.
  4. The search results only contain one of the available bundle option, e.g., in case you can buy 1, 2, or 4 items in a bundle, you might only find the bundle containing 2 items. If you are interested in all bundle options, /pages/bundle-overview-page?sole_article_id=...&show_category_action=true provides that information. The sole_article_id is part of the selling_unit object returned by the search-page-results api. The article ids can be fetched by using the regex /"product-page-bundle-item-(s[0-9]+)"/.
  5. For both, 3 and 4, the x-picnic-agent needs to be part of the request.

Making the first two changes is easy. However, it is hard to maintain the current api when updating point 3 and 4 for the following reasons:

  • The semantics have changed as different bundle options are not part of the search result any more.
  • The data structure has changed, e.g., the discounted price is not part of the selling_unit's decorators any more.
@lazur2006
Copy link

Yes, that is the case. I reversed the API around last week and found a solution based on the new API. Not only the new API endpoint needs to be considered, but also the authentication needs to be changed due to the insufficient authorisations that the old bearer token has. Therefore, the MFA must also be taken into account.

So I made a monkey patch to the Picnic package in advance, but best practice would be to set this up as a default in the latest official Python package. Nevertheless, considering these new things, everything works like a charm.

@Rowaldus
Copy link

Sorry for being a noob but does this mean its being fixed or already fixed? How can i get the monkey patch?

@MRVDH MRVDH self-assigned this Apr 26, 2024
@MRVDH MRVDH added the bug Something isn't working label Apr 26, 2024
@MRVDH
Copy link
Owner

MRVDH commented Apr 27, 2024

Quick update: I'm currently implementing the fix for this, together with some other outdated types and featurs such as the bundling of products and planning to release it tomorrow! 😄

@MRVDH
Copy link
Owner

MRVDH commented Apr 28, 2024

I've added nr 3 in the latest release since it was quite straight forward. For nr 4 I/we need to think about how to do this nicely, as some of the properties have to be extracted from the giant pml objects, which would be very sensitive to errors. Happy to hear your suggestions on this.

@christof-mathies
Copy link
Author

For nr 4 I/we need to think about how to do this nicely, as some of the properties have to be extracted from the giant pml objects, which would be very sensitive to errors. Happy to hear your suggestions on this.

@MRVDH, in my local copy I avoided parsing the whole pml structure by using a regex to find the article ids of the bundle options.

getBundles: (soleArticleId: string) => Promise<string[]> = async (
    soleArticleId: string,
  ) => {
    const picnicClient = await this.createPicnicClient();

    const response: any = await picnicClient.sendRequest(
      "GET",
      `/pages/bundle-overview-page?sole_article_id=${soleArticleId}&show_category_action=true`,
      null,
      true,
    );

    const articleIdMatches: string[] = Array.from(
      JSON.stringify(response.body).matchAll(
        /"product-page-bundle-item-(s[0-9]+)"/g,
      ),
    ).map((match) => match[1]);

    const uniqueArticleIds: string[] = [...new Set(articleIdMatches)];

    return uniqueArticleIds;
  };

Based on the article ids the actual articles can be retrieved using the existing getArticle API. The downside of this approach is that you need to make an additional network request per bundle option. The benefit is that we don't depend on the overall pml structure but only on that one regular expression. If we go with such an approach, soleArticleId needs to be added to the SearchResult type.

@MRVDH
Copy link
Owner

MRVDH commented Apr 29, 2024

Added this in v3.1.1! Let me know if there can be more improvements, otherwise I'd say we can close this issue.

@christof-mathies
Copy link
Author

It is working great - thanks for making the changes!

@thijmen-j
Copy link

@MRVDH, I'd like to have a crack at implementing fixes for this issue in the python-picnic-api as well.

However, looking through the commit d4d616 it is not immediately obvious to me which changes address the 5 points mentioned by @christof-mathies at the beginning of this thread. Could you point me to the right place? (e.g., the x-picnic-agent update)

@MRVDH
Copy link
Owner

MRVDH commented Apr 30, 2024

Hi @thijmen-j , sure!

The commits are a bit messy due to formatting changes, so let me point you to the relevant pieces of code:

  1. https://github.com/MRVDH/picnic-api/blob/v3.1.2/src/index.ts#L79
  2. https://github.com/MRVDH/picnic-api/blob/v3.1.2/src/index.ts#L476
  3. https://github.com/MRVDH/picnic-api/blob/v3.1.2/src/index.ts#L145-L166
  4. https://github.com/MRVDH/picnic-api/blob/v3.1.2/src/index.ts#L172-L192
  5. This is the 4th true parameter when sendReuest is called. For some endpoints it has to be included. (1, 2)

Let me know if that clears it up or if you need more info 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants