Skip to content

feat: add search controller + search and reverseSearch methods #311

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

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

Conversation

TMSchipper
Copy link
Collaborator

No description provided.

@TMSchipper TMSchipper requested a review from boazpoolman August 4, 2025 19:41
Copy link

changeset-bot bot commented Aug 4, 2025

⚠️ No Changeset found

Latest commit: 2bf4b21

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@boazpoolman boazpoolman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @TMSchipper 👋

Thank you for putting up the MR. These 'search' endpoints seem like a nice addition to the plugin.

I've left some comments on your code, please let me know if you have any questions or need clarification about my feedback. In addition to that there are also some linting issues that need to be solved. Primarily the @typescript-eslint rules are giving some trouble.

When you're done, please don't forget to add a Changeset to make sure your change gets scheduled for publication 🙂

}));

// eslint-disable-next-line max-len
results = [...results, ...(Array.isArray(entriesWithContentType) ? entriesWithContentType : [])];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maby it is nicer if we do an early exit when there are no entries after fetching them. Then we don't have to check if entriesWithContentType is an array, we know it is.

reverseSearch: async (ctx: Context & { params: { contentType: string; documentId: string } }) => {
const { contentType, documentId } = ctx.params;

// Zoek met filters via findMany omdat documentId geen primaire ID is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use English comments.

const { contentType, documentId } = ctx.params;

// Zoek met filters via findMany omdat documentId geen primaire ID is
const [entry] = await strapi.entityService.findMany(contentType as UID, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably use the document service here with a findOne.

const value = JSON.parse(coreStoreSettings[0].value);
const { mainField } = value.settings;

const entries = await strapi.entityService.findMany(uid, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably use the document service here.

// Zoek met filters via findMany omdat documentId geen primaire ID is
const [entry] = await strapi.entityService.findMany(contentType as UID, {
filters: { documentId },
fields: ['id', 'title', 'documentId'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't assume the content type you're fetching here will always have a title field. You should use the same technique for finding the 'mainField' as it's done in the search controller.

if (!coreStoreSettings[0]) return;

const value = JSON.parse(coreStoreSettings[0].value);
const { mainField } = value.settings;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I'm thinking about it, it would be nice to make a dedicated service for finding the main field of content type. That should make it easy to reuse the code in both the endpoints.

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