Skip to content

Testing - #220

Merged
ManishBlueprints merged 2 commits into
stagingfrom
testing
Jul 1, 2026
Merged

Testing#220
ManishBlueprints merged 2 commits into
stagingfrom
testing

Conversation

@ManishBlueprints

Copy link
Copy Markdown
Owner

Description

Briefly describe the changes you've made.

Why is this change required?

What problem does it solve?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

ManishBlueprints and others added 2 commits July 2, 2026 00:20
#219)

## Description

Refactor query logic for user access and add Deno configuration file for
new functions

## Why is this change required?

What problem does it solve?

## How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Signed-in users requesting deck content can now also receive public
decks, improving access to shared items.
* **Chores**
* Updated backend function dependencies for better consistency and
runtime reliability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@vercel

vercel Bot commented Jul 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
deckly Ready Ready Preview, Comment Jul 1, 2026 7:00pm

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • testing
  • master

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5dfc3a7e-26b6-4be6-a7fc-9408d43cf2c6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch testing

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Allow signed-in users to access public decks; add Deno import maps

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Allow signed-in users to resolve deck scopes for either owned or public decks.
• Add per-function Deno import maps to standardize Supabase/Std dependencies.
• Align ai-summary function to use the npm Supabase JS v2 import specifier.
Diagram

graph TD
  A["Extract doc text API"] --> B["resolveRecordsForScope"] --> C[("decks table")]
  D["Supabase edge functions"] --> E["deno.json import map"] --> F["supabase-js v2"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Rely on RLS policies (single query by id)
  • ➕ Moves access control to the database layer; fewer app-level conditionals
  • ➕ Reduces risk of query logic drift across call sites
  • ➖ May require policy changes and careful service-role vs user-session separation
  • ➖ Harder to reason about in function contexts that use elevated privileges
2. Two explicit query branches (owned-first, then public fallback)
  • ➕ Avoids PostgREST or semantics and grouping ambiguity
  • ➕ Clearer error messaging for each access mode
  • ➖ Extra round-trip in the fallback case
  • ➖ More code paths to maintain and test

Recommendation: Current approach is reasonable if the underlying PostgREST/Supabase .or() semantics are well understood (filters are ANDed with prior .eq(id, ...)). If future access rules expand, consider migrating authorization to RLS to keep access control centralized.

Files changed (6) +30 / -2

Bug fix (1) +1 / -1
extract-document-text.tsAllow authenticated deck scope resolution for owned OR public decks +1/-1

Allow authenticated deck scope resolution for owned OR public decks

• Adjusts deck scope query filtering so signed-in users can retrieve a deck if they either own it or it is marked public. Keeps guest behavior restricted to public decks only.

api/extract-document-text.ts

Other (5) +29 / -1
deno.jsonAdd Deno import map for ai-summary function dependencies +7/-0

Add Deno import map for ai-summary function dependencies

• Introduces a per-function deno.json import map to pin supabase-js and Deno std module versions for consistent edge runtime resolution.

supabase/functions/ai-summary/deno.json

index.tsSwitch Supabase JS import to npm v2 specifier +1/-1

Switch Supabase JS import to npm v2 specifier

• Updates the supabase-js import to use the Deno npm specifier for v2, aligning with import-map based dependency resolution.

supabase/functions/ai-summary/index.ts

deno.jsonAdd Deno import map for delete-account function dependencies +7/-0

Add Deno import map for delete-account function dependencies

• Adds deno.json imports for supabase-js and required Deno std libraries to standardize dependency resolution.

supabase/functions/delete-account/deno.json

deno.jsonAdd Deno import map for r2-storage function dependencies +7/-0

Add Deno import map for r2-storage function dependencies

• Adds deno.json imports to pin supabase-js and Deno std versions for the r2-storage edge function.

supabase/functions/r2-storage/deno.json

deno.jsonAdd Deno import map for sign-deck-url function dependencies +7/-0

Add Deno import map for sign-deck-url function dependencies

• Adds deno.json imports to ensure consistent dependency resolution for the sign-deck-url edge function.

supabase/functions/sign-deck-url/deno.json

@ManishBlueprints
ManishBlueprints merged commit 43350ce into staging Jul 1, 2026
9 checks passed
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Inconsistent Supabase imports 🐞 Bug ⚙ Maintainability
Description
The ai-summary Edge function imports supabase-js via a direct npm: specifier while transitive code
imports it via "@supabase/supabase-js" (resolved through deno.json), creating avoidable
resolution/version-skew risk within the same function bundle. This makes dependency upgrades harder
and can lead to duplicate resolved entries if the constraints don’t perfectly dedupe.
Code

supabase/functions/ai-summary/index.ts[1]

+import { createClient, type SupabaseClient } from "npm:@supabase/supabase-js@2";
Relevance

⭐ Low

PR #221 merged same npm:@supabase/supabase-js@2 import change; no evidence team enforces consistent
alias usage.

PR-#221

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The ai-summary function directly imports npm:@supabase/supabase-js@2, but a transitive dependency
path imports @supabase/supabase-js, and the function’s import map pins that alias to a different
specifier; this is concrete inconsistency within the same function graph.

supabase/functions/ai-summary/index.ts[1-1]
supabase/functions/ai-summary/deno.json[1-7]
src/services/aiRetrievalQueryService.ts[1-5]
src/services/supabase.ts[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`supabase/functions/ai-summary/index.ts` imports Supabase using a direct `npm:` specifier, while other modules used by this function import Supabase via `@supabase/supabase-js` (which is intended to be controlled by the function’s `deno.json` import map). This inconsistency can cause version-skew/duplication risk and defeats centralized dependency management.

## Issue Context
- `ai-summary/deno.json` already defines an import map for `@supabase/supabase-js`, but `index.ts` bypasses it.
- Transitive imports (via `src/services/aiRetrievalQueryService.ts` -> `src/services/supabase.ts`) use `@supabase/supabase-js`.

## Fix Focus Areas
- supabase/functions/ai-summary/index.ts[1-1]
- supabase/functions/ai-summary/deno.json[1-7]

## Suggested change
- In `supabase/functions/ai-summary/index.ts`, change the import to `import { createClient, type SupabaseClient } from "@supabase/supabase-js";` so it uses the import map.
- Ensure the desired version is controlled in `supabase/functions/ai-summary/deno.json` (and/or standardized across functions).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Outdated Deno lockfile 🐞 Bug ☼ Reliability
Description
New per-function deno.json import maps switch Edge functions to
npm:@supabase/supabase-js@^2.100.1, but supabase/functions/deno.lock still pins the old
https://esm.sh/@supabase/supabase-js@2 redirect set. This makes the lockfile
misleading/out-of-sync and undermines reproducibility if you rely on it for deterministic dependency
resolution.
Code

supabase/functions/delete-account/deno.json[R1-7]

+{
+  "imports": {
+    "@supabase/supabase-js": "npm:@supabase/supabase-js@^2.100.1",
+    "@std/http": "jsr:@std/http@0.224.5",
+    "@std/encoding": "jsr:@std/encoding@0.224.3"
+  }
+}
Relevance

⭐ Low

PR #221 introduced per-function import maps/npm specifiers without updating
supabase/functions/deno.lock; suggests lockfile not enforced.

PR-#221

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds function-level import maps targeting npm specifiers, while the existing lockfile clearly
pins esm.sh URLs for supabase-js; these two sources of truth are now inconsistent.

supabase/functions/delete-account/deno.json[1-7]
supabase/functions/deno.lock[1-25]
supabase/functions/deno.json[1-7]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The repository contains `supabase/functions/deno.lock` that pins `esm.sh` Supabase JS URLs, but the PR introduces per-function `deno.json` import maps pointing `@supabase/supabase-js` to `npm:` specifiers. This leaves the committed lockfile out of sync with the dependency graph.

## Issue Context
- The lockfile currently captures redirects/remotes for `https://esm.sh/@supabase/supabase-js@2`.
- The new per-function import maps resolve `@supabase/supabase-js` to `npm:@supabase/supabase-js@^2.100.1`.

## Fix Focus Areas
- supabase/functions/delete-account/deno.json[1-7]
- supabase/functions/deno.lock[1-25]
- supabase/functions/deno.json[1-7]

## Suggested change
Pick one consistent strategy:
1) If you want lockfile-based determinism: regenerate/update `supabase/functions/deno.lock` so it reflects the new `npm:`-based resolution (and ensure the same import-map approach is used across functions).
2) If the lockfile is not used: remove `supabase/functions/deno.lock` (or document that it is intentionally stale/not enforced) to avoid future confusion.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

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.

1 participant