Skip to content

Conversation

@greg-in-a-box
Copy link
Contributor

@greg-in-a-box greg-in-a-box commented Aug 29, 2025

when selecting folders or files via the filter chip, they will be added to the query correctly now

Summary by CodeRabbit

  • New Features

    • Filtering now expands logical file-type selections into their mapped extensions and supports combining folder-type and file-type filters in a single query; introduces explicit file/folder type mappings to improve matching.
  • Bug Fixes

    • Standardized mimetype-filter behavior for mixed folder/file selections and field names with special characters; unmapped values now yield no matches.
  • Tests

    • Expanded coverage for mapped types, folder+file combos, indexing offsets, special-character field names, and edge cases.

@greg-in-a-box greg-in-a-box requested a review from a team as a code owner August 29, 2025 15:38
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 29, 2025

Walkthrough

I pity the fool — this PR replaces a mimetype special-case with mapping via mapFileTypes, rewrites mime-type filter construction to emit folder/type and extension clauses (combined with OR when needed), converts a default constant to named exports, and adds/updates tests to cover the new mappings and behaviors.

Changes

Cohort / File(s) Summary of Changes
Query builder logic
src/elements/content-explorer/MetadataQueryBuilder.ts
Removed mimetype-filter special-case in getSelectFilter. Rewrote getMimeTypeFilter to call mapFileTypes, partition folder vs. extension targets, generate sanitized param keys, emit item.extension IN (...) and/or item.type = :... clauses, combine with OR when needed, and return queryParams, queries, and keysGenerated.
Constants
src/elements/content-explorer/constants.ts
Switched CONTENT_EXPLORER_FOLDER_FIELDS_TO_FETCH to a named export; added NON_FOLDER_FILE_TYPES_MAP, FILE_FOLDER_TYPES_MAP (adds folderType), and NON_FOLDER_FILE_TYPES named exports.
Utilities
src/elements/content-explorer/utils.ts
Added mapFileTypes(selectedFileTypes: BoxItemSelection) to map selection keys to file/folder extension lists; returns [], ['file'], or flattened mapped types depending on selection.
Content explorer usage
src/elements/content-explorer/ContentExplorer.tsx
Updated import to use named CONTENT_EXPLORER_FOLDER_FIELDS_TO_FETCH (import style change only).
Tests — query builder & API helper
src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts, src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts
Removed the old mimetype-filter special-case test; added/updated tests to validate mapFileTypes mappings, folder-only filters, folder+file OR combinations, mixed valid/invalid inputs, sanitized field names, custom argIndexStart behavior, and expanded IN-clause parameter expectations.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant MQB as MetadataQueryBuilder
  participant MAP as mapFileTypes
  participant QB as QueryConsumer

  C->>MQB: getMimeTypeFilter(fieldKey, values, argIndexStart)
  MQB->>MAP: mapFileTypes(values)
  MAP-->>MQB: ['folder'] or ['pdf','doc',...] or ['file'] or []
  MQB->>MQB: Partition -> folderPart?, extensionList[]
  alt extensionList non-empty
    MQB->>MQB: gen param keys -> :arg_field_ext_N...
    MQB->>MQB: Clause A: item.extension IN (:args...)
  end
  alt folderPart present
    MQB->>MQB: gen param key -> :arg_mime_folderType_X
    MQB->>MQB: Clause B: item.type = :arg_mime_folderType_X
  end
  alt Both parts exist
    MQB->>MQB: Combine -> (Clause A) OR (Clause B)
  end
  MQB-->>C: { queries, queryParams, keysGenerated }
  C->>QB: apply queries & params
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Possibly related PRs

Suggested labels

ready-to-merge

Suggested reviewers

  • tjuanitas
  • tjiang-box
  • jfox-box

Poem

I pity the fool who trusts one mime rule,
We map types, split folder from file, keep params cool.
Keys sanitized, ORs aligned, tests watch the door,
Ship it clean, ship it mean — now code ready for more. 🚢✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1)

405-510: Solid coverage on folderType OR logic—nice hustle! Add one more edge case, fool.

  • Great tests for mixed values, indexing, and special chars.
  • Add a case for folderType-only with a non-zero start index to lock indexing (e.g., start=5 ⇒ arg_mime_folderType_6).

Example:

test('should handle folderType only with non-zero start index', () => {
    const result = getMimeTypeFilter(['folderType'], 'mimetype', 5);
    expect(result).toEqual({
        queryParams: { arg_mime_folderType_6: 'folder' },
        queries: ['(item.type = :arg_mime_folderType_6)'],
        keysGenerated: 1,
    });
});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP (Model Context Protocol) integration is disabled
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 59e30e3 and 8f6207b.

📒 Files selected for processing (2)
  • src/elements/content-explorer/MetadataQueryBuilder.ts (2 hunks)
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.

Applied to files:

  • src/elements/content-explorer/MetadataQueryBuilder.ts
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts
🧬 Code graph analysis (1)
src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)
  • getMimeTypeFilter (126-187)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint_test_build
  • GitHub Check: Summary
🔇 Additional comments (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)

118-121: Safe to drop the mimetype-filter mapping, fool! No callers invoke getSelectFilter('mimetype-filter') (verified via ripgrep); all mimetype filtering now lives in MetadataQueryAPIHelper.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)

136-141: Good cleanup — no noisy logs left, fool.

Previous debug logging around these paths is gone. Keep it that way.

🧹 Nitpick comments (2)
src/elements/content-explorer/MetadataQueryBuilder.ts (2)

135-150: Solid folder detection; tighten symbols, fool.

Logic’s correct and idempotent for repeated folderType. Consider avoiding magic strings.

-    let hasFolder = false;
+    const FOLDER_TOKEN = 'folderType';
+    const FOLDER_VALUE = 'folder';
+    let hasFolder = false;
@@
-        if (value === 'folderType' && !hasFolder) {
+        if (value === FOLDER_TOKEN && !hasFolder) {
@@
-            queryParams[folderArgKey] = 'folder';
+            queryParams[folderArgKey] = FOLDER_VALUE;
             queries.push(`(item.type = :${folderArgKey})`);
             hasFolder = true;
-        } else if (value !== 'folderType') {
+        } else if (value !== FOLDER_TOKEN) {

157-172: Deduplicate extensions before building IN list, sucka.

Repeated types waste args and can bloat queries. Filter to unique extensions while preserving order.

-    if (extensions.length > 0) {
-        const extensionQueryParams = Object.fromEntries(
-            extensions.map(extension => {
+    if (extensions.length > 0) {
+        const uniqueExtensions = Array.from(new Set(extensions));
+        const extensionQueryParams = Object.fromEntries(
+            uniqueExtensions.map(extension => {
                 currentArgIndex += 1;
                 return [generateArgKey(fieldKey, currentArgIndex), extension];
             }),
         );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP (Model Context Protocol) integration is disabled
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8f6207b and 94c0708.

📒 Files selected for processing (2)
  • src/elements/content-explorer/MetadataQueryBuilder.ts (2 hunks)
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.

Applied to files:

  • src/elements/content-explorer/MetadataQueryBuilder.ts
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts
🧬 Code graph analysis (2)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)
src/api/uploads/BaseUpload.js (1)
  • extension (178-178)
src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)
  • getMimeTypeFilter (126-182)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint_test_build
  • GitHub Check: Summary
🔇 Additional comments (3)
src/elements/content-explorer/MetadataQueryBuilder.ts (3)

174-176: LGTM on OR-wrapping.

Parentheses and OR-composition are correct and produce a single clause when needed.


97-124: Drop the mimetype-filter guard in getSelectFilter — I pity the fool who adds dead code; the API helper already delegates 'mimetype-filter' to getMimeTypeFilter.

Likely an incorrect or invalid review comment.


168-171: Ignore dot-prefix concern – extensions are consistently normalized without leading dots (FILE_EXT_REGEX in utils/file.js and ContentUploader logic), and MetadataQueryBuilder queries item.extension with dotless values. I pity the fool who doubts it.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)

99-126: Back-compat regression: honor ‘mimetype-filter’ in getSelectFilter, fool.

Per previous learnings, calls passing 'mimetype-filter' must still work. Delegate to getMimeTypeFilter to avoid breaking callers.

Apply this diff:

 export const getSelectFilter = (filterValue: string[], fieldKey: string, argIndexStart: number): QueryResult => {
+    // Back-compat: delegate mimetype-filter to mime-specific handler
+    if (fieldKey === 'mimetype-filter') {
+        return getMimeTypeFilter(filterValue, fieldKey, argIndexStart);
+    }
     if (!Array.isArray(filterValue) || filterValue.length === 0) {
         return {
             queryParams: {},
             queries: [],
             keysGenerated: 0,
         };
     }
♻️ Duplicate comments (1)
src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1)

414-533: Add back-compat test for getSelectFilter('mimetype-filter'), fool.

We asked for this before; it guards regressions when callers still use getSelectFilter.

Apply this diff:

@@
     describe('getMimeTypeFilter', () => {
@@
     });
+
+    // Back-compat: ensure getSelectFilter delegates for 'mimetype-filter'
+    describe('getSelectFilter (mimetype-filter back-compat)', () => {
+        test('should delegate to getMimeTypeFilter and support folderType OR extension', () => {
+            const result = getSelectFilter(['folderType', 'pdfType'], 'mimetype-filter', 0);
+            expect(result).toEqual({
+                queryParams: {
+                    arg_mime_folderType_1: 'folder',
+                    arg_mimetype_filter_2: 'pdf',
+                },
+                queries: ['((item.type = :arg_mime_folderType_1) OR (item.extension IN (:arg_mimetype_filter_2)))'],
+                keysGenerated: 2,
+            });
+        });
+    });
🧹 Nitpick comments (4)
src/elements/content-explorer/constants.ts (4)

13-47: Broaden common types (optional) — don’t make users feel foolish.

Consider covering widely used formats you’re currently missing.

 export const NON_FOLDER_FILE_TYPES_MAP = new Map([
   ['boxnoteType', ['boxnote']],
   ['boxcanvasType', ['boxcanvas']],
   ['pdfType', ['pdf']],
   ['documentType', ['doc', 'docx', 'gdoc', 'rtf', 'txt']],
   ['spreadsheetType', ['xls', 'xlsx', 'xlsm', 'csv', 'gsheet']],
   ['presentationType', ['ppt', 'pptx', 'odp']],
-  ['imageType', ['png', 'jpg', 'jpeg', 'gif', 'bmp', 'tif', 'tiff']],
-  ['audioType', ['mp3', 'm4a', 'm4p', 'wav', 'mid', 'wma']],
+  ['imageType', ['png', 'jpg', 'jpeg', 'gif', 'bmp', 'tif', 'tiff', 'webp', 'svg', 'heic', 'heif']],
+  ['audioType', ['mp3', 'm4a', 'm4p', 'wav', 'mid', 'wma', 'aac', 'flac', 'opus', 'aiff', 'aif', 'oga']],
   [
     'videoType',
     [
       'mp4',
       'mpeg',
       'mpg',
       'wmv',
       '3g2',
       '3gp',
       'avi',
       'm2v',
       'm4v',
       'mkv',
       'mov',
-      'ogg',
+      'ogg',  // container; keep if you must, but consider 'ogv' for video and 'oga' for audio
+      'webm',
       'mts',
       'qt',
       'ts',
       'flv',
       'rm',
     ],
   ],
   ['drawingType', ['dwg', 'dxf']],
-  ['threedType', ['obj', 'fbx', 'stl', 'amf', 'iges']],
+  ['threedType', ['obj', 'fbx', 'stl', 'amf', 'iges', 'igs', 'step', 'stp']],
 ]);

Optional: If extension matching is case-insensitive elsewhere, ensure normalization to lowercase before comparison; otherwise you’ll look like a fool when 'JPG' slips through.


13-47: Add explicit types and immutability — lock it down, chump.

Make intent clear and prevent accidental mutation by typing these as read-only.

-export const NON_FOLDER_FILE_TYPES_MAP = new Map([
+export const NON_FOLDER_FILE_TYPES_MAP: ReadonlyMap<string, readonly string[]> =
+  new Map<string, readonly string[]>([
     ['boxnoteType', ['boxnote']],
     ['boxcanvasType', ['boxcanvas']],
     ['pdfType', ['pdf']],
     ['documentType', ['doc', 'docx', 'gdoc', 'rtf', 'txt']],
     ['spreadsheetType', ['xls', 'xlsx', 'xlsm', 'csv', 'gsheet']],
     ['presentationType', ['ppt', 'pptx', 'odp']],
     ['imageType', ['png', 'jpg', 'jpeg', 'gif', 'bmp', 'tif', 'tiff']],
     ['audioType', ['mp3', 'm4a', 'm4p', 'wav', 'mid', 'wma']],
     [
       'videoType',
       [
         'mp4',
         'mpeg',
         'mpg',
         'wmv',
         '3g2',
         '3gp',
         'avi',
         'm2v',
         'm2v',
         'm4v',
         'mkv',
         'mov',
         'ogg',
         'mts',
         'qt',
         'ts',
         'flv',
         'rm',
       ],
     ],
     ['drawingType', ['dwg', 'dxf']],
     ['threedType', ['obj', 'fbx', 'stl', 'amf', 'iges']],
-]);
+  ] as const);

49-49: Don’t chain .set — build the map immutably and type it, fool.

Clearer construction and read-only typing avoid accidental mutation.

-export const FILE_FOLDER_TYPES_MAP = new Map(NON_FOLDER_FILE_TYPES_MAP).set('folderType', ['folder']);
+export const FILE_FOLDER_TYPES_MAP: ReadonlyMap<string, readonly string[]> =
+  new Map<string, readonly string[]>([
+    ...NON_FOLDER_FILE_TYPES_MAP,
+    ['folderType', ['folder'] as const],
+  ]);

51-51: Type-safe keys export — give consumers a union, not guesses.

Export keys as a readonly tuple and a union type. Don’t make your users parse maps like fools.

-export const NON_FOLDER_FILE_TYPES = Array.from(NON_FOLDER_FILE_TYPES_MAP.keys());
+export const NON_FOLDER_FILE_TYPES = [...NON_FOLDER_FILE_TYPES_MAP.keys()] as const;
+export type NonFolderFileType = typeof NON_FOLDER_FILE_TYPES[number];

If you need perfect literal inference, consider defining the mapping as a const object and deriving both the Map and the key union from it.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP (Model Context Protocol) integration is disabled
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 94c0708 and 90e8ec6.

📒 Files selected for processing (6)
  • src/elements/content-explorer/ContentExplorer.tsx (1 hunks)
  • src/elements/content-explorer/MetadataQueryBuilder.ts (3 hunks)
  • src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts (1 hunks)
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (3 hunks)
  • src/elements/content-explorer/constants.ts (1 hunks)
  • src/elements/content-explorer/utils.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.

Applied to files:

  • src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts
  • src/elements/content-explorer/utils.ts
  • src/elements/content-explorer/MetadataQueryBuilder.ts
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts
🧬 Code graph analysis (4)
src/elements/content-explorer/utils.ts (1)
src/elements/content-explorer/constants.ts (2)
  • NON_FOLDER_FILE_TYPES (51-51)
  • FILE_FOLDER_TYPES_MAP (49-49)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)
src/elements/content-explorer/utils.ts (1)
  • mapFileTypes (199-212)
src/elements/content-explorer/constants.ts (1)
src/utils/fields.js (1)
  • FOLDER_FIELDS_TO_FETCH (66-85)
src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)
  • getMimeTypeFilter (128-194)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint_test_build
  • GitHub Check: Summary
🔇 Additional comments (6)
src/elements/content-explorer/ContentExplorer.tsx (1)

37-37: Named import switch looks right, fool.

Import matches the new named export; no runtime impact expected.

src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts (1)

676-690: Tests align with expanded mime-type mapping.

Good expectation updates for mapped extensions and arg keys.

src/elements/content-explorer/MetadataQueryBuilder.ts (1)

137-194: No callers pass ‘mimetype-filter’ into getSelectFilter—I pity the fool who thought otherwise!

src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (1)

298-316: LGTM on mapFileTypes-based expectations.

Expanded IN list and arg indexing look consistent.

src/elements/content-explorer/constants.ts (2)

5-11: Named export safe—no default imports found
I pity the fool worrying ’bout breakage—ran a repo-wide search and found zero default imports of this module.


13-51: MIME type filter special-case mapping remains intact.
I pity the fool who breaks query semantics—getMimeTypeFilter still maps selected types to item.extension IN (…) for file extensions and uses item.type = 'folder' for folderType.

tjiang-box
tjiang-box previously approved these changes Aug 29, 2025
jpan-box
jpan-box previously approved these changes Aug 29, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
src/elements/content-explorer/MetadataQueryBuilder.ts (2)

128-135: Hoist getMimeTypeFilter to allow delegation from getSelectFilter, fool.

Switch to a function declaration so the earlier guard can call it without TDZ issues.

-export const getMimeTypeFilter = (filterValue: string[], fieldKey: string, argIndexStart: number): QueryResult => {
+export function getMimeTypeFilter(filterValue: string[], fieldKey: string, argIndexStart: number): QueryResult {

And close with } instead of }; at the end of the function.

-};
+}

99-126: Add mimetype-filter delegation in getSelectFilter
I pity the fool who drops the legacy mapping—preserve it by inserting before the multi-select logic:

 export const getSelectFilter = (filterValue: string[], fieldKey: string, argIndexStart: number): QueryResult => {
     if (!Array.isArray(filterValue) || filterValue.length === 0) {
         return {
             queryParams: {},
             queries: [],
             keysGenerated: 0,
         };
     }
+    // Delegate legacy 'mimetype-filter' to the MIME-specific builder
+    if (fieldKey === 'mimetype-filter') {
+        return getMimeTypeFilter(filterValue, fieldKey, argIndexStart);
+    }
 
     let currentArgIndex = argIndexStart;
     …
♻️ Duplicate comments (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)

137-167: Handle ‘file’ sentinel and de-dupe mapped values, or your query gonna be bogus.

mapFileTypes can return 'file' and 'folder'. You only handle 'folder', leading to item.extension IN ('file') nonsense. Also de-dupe to keep args tight.

-    // Use mapFileTypes to get the correct extensions and handle special cases
-    const mappedExtensions = mapFileTypes(filterValue as BoxItemSelection);
-    if (mappedExtensions.length === 0) {
+    // Use mapFileTypes to get the correct extensions/types and handle special cases
+    const mapped = Array.from(new Set(mapFileTypes(filterValue as BoxItemSelection)));
+    if (mapped.length === 0) {
         return {
             queryParams: {},
             queries: [],
             keysGenerated: 0,
         };
     }
 
     let currentArgIndex = argIndexStart;
     const queryParams: { [key: string]: number | Date | string } = {};
     const queries: string[] = [];
 
     // Handle specific extensions and folder type
     const extensions: string[] = [];
     let hasFolder = false;
+    let hasFile = false;
 
-    for (const extension of mappedExtensions) {
+    for (const extension of mapped) {
         if (extension === 'folder') {
             if (!hasFolder) {
                 currentArgIndex += 1;
                 const folderArgKey = generateArgKey('mime_folderType', currentArgIndex);
                 queryParams[folderArgKey] = 'folder';
                 queries.push(`(item.type = :${folderArgKey})`);
                 hasFolder = true;
             }
+        } else if (extension === 'file') {
+            if (!hasFile) {
+                currentArgIndex += 1;
+                const fileArgKey = generateArgKey('mime_fileType', currentArgIndex);
+                queryParams[fileArgKey] = 'file';
+                queries.push(`(item.type = :${fileArgKey})`);
+                hasFile = true;
+            }
         } else {
             extensions.push(extension);
         }
     }
🧹 Nitpick comments (2)
src/elements/content-explorer/MetadataQueryBuilder.ts (2)

1-1: Use a type-only import for BoxItemSelection, fool.

Avoid pulling runtime code when you only need a type.

-import { BoxItemSelection } from '@box/box-item-type-selector';
+import type { BoxItemSelection } from '@box/box-item-type-selector';

120-123: Field key safety: ensure only trusted keys hit HASANY, sucka.

Using raw fieldKey in the query is fine for internal constants, but don’t feed user input here. If any dynamic mapping exists upstream, validate against an allowlist.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP (Model Context Protocol) integration is disabled
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 90e8ec6 and 32802e3.

📒 Files selected for processing (6)
  • src/elements/content-explorer/ContentExplorer.tsx (1 hunks)
  • src/elements/content-explorer/MetadataQueryBuilder.ts (3 hunks)
  • src/elements/content-explorer/__tests__/MetadataQueryAPIHelper.test.ts (1 hunks)
  • src/elements/content-explorer/__tests__/MetadataQueryBuilder.test.ts (3 hunks)
  • src/elements/content-explorer/constants.ts (1 hunks)
  • src/elements/content-explorer/utils.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/elements/content-explorer/ContentExplorer.tsx
  • src/elements/content-explorer/constants.ts
  • src/elements/content-explorer/tests/MetadataQueryAPIHelper.test.ts
  • src/elements/content-explorer/utils.ts
  • src/elements/content-explorer/tests/MetadataQueryBuilder.test.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.

Applied to files:

  • src/elements/content-explorer/MetadataQueryBuilder.ts
🧬 Code graph analysis (1)
src/elements/content-explorer/MetadataQueryBuilder.ts (1)
src/elements/content-explorer/utils.ts (1)
  • mapFileTypes (199-212)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: lint_test_build
  • GitHub Check: Summary
🔇 Additional comments (3)
src/elements/content-explorer/MetadataQueryBuilder.ts (3)

169-184: Batching IN clause looks solid.

With deduped inputs from the previous fix, this is clean and efficient.


186-193: OR-combine is correct and parenthesized properly.

This preserves precedence when mixing folder/file type checks with extension IN.


99-126: No direct getSelectFilter misuse found. I pity the fool who doubts—grep confirms all “mimetype-filter” keys go through getMimeTypeFilter in MetadataQueryAPIHelper, with no callers of getSelectFilter using it.

@mergify mergify bot merged commit bdd6d1d into box:master Aug 29, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants