-
Notifications
You must be signed in to change notification settings - Fork 59
feat(ci): trigger preview release on master push #425
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
base: master
Are you sure you want to change the base?
Conversation
* feat(query-mikro-orm): add MikroORM adapter package Add @ptc-org/nestjs-query-mikro-orm package providing MikroORM integration for nestjs-query. This includes: - MikroOrmQueryService implementing QueryService interface - Filter conversion from nestjs-query to MikroORM operators - Support for sorting with null handling - Relation queries (findRelation, queryRelations, countRelations) - NestjsQueryMikroOrmModule with forFeature() pattern - Factory providers for automatic service registration - MikroOrmAssembler for DTO/Entity conversion - Comprehensive test suite with 46 tests Closes TriPSs#178 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix(query-mikro-orm): add eslint config and fix lint errors - Add missing .eslintrc.json for package - Move function definition before usage to fix no-use-before-define - Prefix unused parameters with underscore - Auto-fix formatting issues from prettier/eslint 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix(ci): include node version in cache key for native modules The better-sqlite3 native module needs to be compiled per Node version. Previous cache key only used yarn.lock hash, causing NODE_MODULE_VERSION mismatch errors when running tests on Node 22.x and 23.x. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * mikro-orm: what about opts in findRelationForEntity ? (vibe-kanban 34a5f9d7) throw an error if something is passed that we don't support, e.g. filtering (are there any other options that we silently ignore?) * Fix MikroOrmQueryService to allow empty filter objects on findRelation (vibe-kanban c60da658) ## Problem When using `@Authorize` decorator with a `CustomAuthorizer` that returns an empty filter `{}`, the `findRelationForEntity` method throws: ``` Error: MikroOrmQueryService does not support filtering on findRelation ``` This happens because the check `if (opts?.filter)` is truthy for empty objects `{}`. ## Root Cause In `packages/query-mikro-orm/src/services/mikro-orm-query.service.ts`, line ~187: ```typescript if (opts?.filter) { throw new Error('MikroOrmQueryService does not support filtering on findRelation') } ``` An empty filter `{}` is truthy, so it triggers the error even though there are no actual filter conditions. ## Solution Change the check to verify the filter has actual conditions: ```typescript if (opts?.filter && Object.keys(opts.filter).length > 0) { throw new Error('MikroOrmQueryService does not support filtering on findRelation') } ``` ## Related This affects any DTO using `@Authorize` with a `CustomAuthorizer` that returns `{}` when accessing `@Relation` fields. * mikro-orm: add filtering support to findRelation (vibe-kanban 8664fc12) --------- Co-authored-by: Claude <[email protected]>
This allows pkg.pr.new packages to be published when commits are merged to master, not just for pull requests.
📝 WalkthroughWalkthroughA GitHub Actions workflow configuration update that extends the publish-preview workflow trigger conditions. The workflow now executes on direct pushes to the master branch in addition to existing pull request and manual workflow dispatch triggers, enabling automated publishing on master commits. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit b95f2a7
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish-preview.yml (1)
5-12: Add a fallback forNX_BRANCHon push/workflow_dispatch events.
github.event.numberis only available for pull_request events. On push and workflow_dispatch triggers, it will be undefined, leavingNX_BRANCHempty. This can impact Nx grouping/caching or break logic expecting a non-empty branch identifier. Usegithub.ref_nameas a fallback:🔧 Suggested adjustment
env: - NX_BRANCH: ${{ github.event.number }} + NX_BRANCH: ${{ github.event.number || github.ref_name }} NX_RUN_GROUP: ${{ github.run_id }}
|
BTW, @TriPSs could you enable pkg.pr.new gh app for this repo? So that the workflow actually works. And, maybe make a release? So that mikro-orm query service is in the official release. |
|
Tried to do a release this morning but NPM changed the way the tokens work and I had not enough time yet to fix it. Will do a release soon locally so it's available. |
|
OK. How about enabling support for pkg.pr.new ? All you need is just to give it access to this repo in github. (If you don't want to do this, then I say that we should remove .github/workflows/publish-preview.yml , as it's simply not working right now. ) |
This allows pkg.pr.new packages to be published when commits are merged to master, not just for pull requests.
This enables consumers to use the latest master branch packages via pkg.pr.new without waiting for an official npm release.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.