Skip to content

feat: extend sideload #1099

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

Merged
merged 1 commit into from
Apr 14, 2025
Merged

feat: extend sideload #1099

merged 1 commit into from
Apr 14, 2025

Conversation

adamcikado
Copy link
Contributor

@adamcikado adamcikado commented Mar 23, 2025

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR extends sideload functionality:

  • Changes sideloaded on query builder to public so it can be retrieved when querying.
  • Infers custom sideloaded type in query builder.
  • Adds option to only merge sideloaded data, not replace it completely.

On some projects we use sideload for getting translation data for localized content. When we filter the content by search phrase we need to check what locale (set in sideloaded) is used for the query to run the condition for correct locale. In order for this to work we need to be able to retrieve sideloaded in query builder. It's also very helpful to be able to add extra data to already sideloaded data, when there is other chained functionality, so it won't get replaced.
This PR enhances the sideload functionality to support it.

PS: This PR also uses @japa/expect-type, so I based it off #1097 and it should be merged first.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@adamcikado adamcikado changed the title feat feat: extend sideload Mar 23, 2025
@thetutlage
Copy link
Member

Seems like this PR has commits from other PR as well. Can you please fix that?

@adamcikado
Copy link
Contributor Author

@thetutlage it's rebased!

@thetutlage thetutlage merged commit a80678a into adonisjs:21.x Apr 14, 2025
16 checks passed
@thetutlage
Copy link
Member

Looks good.

On the side note, I would love to understand the flow you have around searching by the current locale and see if this is something that could be solved in a more intuitive manner.

@adamcikado
Copy link
Contributor Author

adamcikado commented Apr 18, 2025

@thetutlage Thanks!

We use it in following manner (simplified):

function LocalizableTrait(Base) {
  class LocalizableMixin extends Base {
    @hasMany(() => Translation)
    declare translations: HasMany<typeof Translation>

    static locale = scope((query, locale) => {
      query.sideloaded.locale = locale

      query.preload('translations', (translationQuery) => translationQuery.where('locale', locale))
    })
  }

  return LocalizableMixin
}

class Page extends compose(BaseModel, LocalizableTrait) {
  @manyToMany(() => PageCategory)
  declare categories: ManyToMany<typeof PageCategory>
}

class PageCategory extends compose(BaseModel, LocalizableTrait) {
}

// somewhere in the repository
const query = Page.query().withScopes(scopes => scopes.locale('en'))

// then when we pass the query to other methods for preloading or filtering we can use:
query.preload('categories', categoriesQuery => {
  categoriesQuery.withScopes(scopes => scopes.locale(query.sideloaded.locale))
})

query.whereHas('translations', (translationQuery) => {
    translationQuery.where('locale', query.sideloaded.locale)
      .whereRaw('fulltext @@ plainto_tsquery('simple', ?))', [text])
})

@thetutlage
Copy link
Member

I see. I think it will be helpful to have some hooks around preloads so that these nested withScopes can be applied automatically (if you want that). However, being explicit is also fine

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