Skip to content
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

Support dimensions filter or,and,not group #128

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

lcc3108
Copy link
Member

@lcc3108 lcc3108 commented Oct 19, 2024

Summary

support dimensions filter and,or,not group

#98
#76

Summary by CodeRabbit

  • 새로운 기능

    • DimensionFilter 컴포넌트가 GAFilterExpressionComponent로 이름이 변경되고, 필터 표현식을 관리하는 방식이 개선되었습니다.
    • 새로운 프로퍼티 onDeleteselectedDimensions가 추가되었습니다.
    • QueryEditorGA4 컴포넌트에서 새로운 GAFilterExpressionComponent를 사용하여 필터 표현식 관리가 개선되었습니다.
  • 버그 수정

    • QueryEditorGA4 컴포넌트에서 필터 표현식 관리 방식이 간소화되었습니다.
  • 문서화

    • GAFilter 인터페이스의 filterType 속성이 GADimensionFilterType으로 변경되어 필터의 타입이 더 명확해졌습니다.

Copy link
Contributor

coderabbitai bot commented Oct 19, 2024

Walkthrough

이 변경 사항은 DimensionFilter 컴포넌트를 GAFilterExpressionComponent로 이름을 변경하고, 새로운 프로퍼티와 내부 상태 관리를 도입하여 필터 표현식을 관리하는 방식을 개선합니다. QueryEditorGA4 컴포넌트는 새로운 필터 컴포넌트를 사용하도록 업데이트되었으며, GAFilter 인터페이스의 filterType 속성이 GADimensionFilterType으로 변경되었습니다. 이러한 변경은 필터 관리의 유연성과 명확성을 높이는 방향으로 진행되었습니다.

Changes

파일 변경 요약
src/Filter.tsx - DimensionFilter에서 GAFilterExpressionComponent로 이름 변경
- 새로운 프로퍼티 onDelete, selectedDimensions 추가
- 내부 상태 관리 제거 및 필터 표현식 처리 로직 개편
- 필터 렌더링 로직 모듈화 및 UI 구성 요소 업데이트
src/QueryEditorGA4.tsx - DimensionFilterGAFilterExpressionComponent로 변경
- onFiltersExpressionChange 메서드 시그니처 업데이트 및 로직 단순화
- query 객체 초기화 방식 조정
src/types.ts - GAFilter 인터페이스의 filterType 속성을 GADimensionFilterType으로 변경

Poem

🐰 변화의 바람이 불어와
필터가 새롭게 태어났네,
표현식의 춤을 추며
더 유연하게, 더 명확하게,
함께하는 길을 찾아가요,
Rabbit과 함께하는 이 길,
즐거운 필터의 세계로! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ 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.
    • Generate unit testing code for this file.
    • 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. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai 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

Documentation and Community

  • 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: 2

🧹 Outside diff range and nitpick comments (1)
src/QueryEditorGA4.tsx (1)

324-329: 'onDelete' 속성은 생략이 가능할 수 있습니다.

'GAFilterExpressionComponent'에 'onDelete={undefined}'를 명시적으로 전달하고 있습니다. 만약 'onDelete' 속성이 선택적(optional)인 경우, 해당 속성을 생략해도 무방합니다.

 <GAFilterExpressionComponent 
   expression={query.dimensionFilter} 
   onChange={this.onFiltersExpressionChange}
   selectedDimensions={selectedDimensions}
-  onDelete={undefined}
 />
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between bc73543 and 4cd9505.

📒 Files selected for processing (3)
  • src/Filter.tsx (1 hunks)
  • src/QueryEditorGA4.tsx (3 hunks)
  • src/types.ts (1 hunks)
🧰 Additional context used
🪛 Biome
src/Filter.tsx

[error] 129-129: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 133-133: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🔇 Additional comments (4)
src/types.ts (1)

101-103: filterType 속성 변경 확인 필요

GAFilter 인터페이스의 filterType 속성이 GADimensionFilterType으로 변경된 것은 PR의 목적에 부합합니다.

그러나 다음 사항들을 확인해 주시기 바랍니다:

  1. GAMetricFilterType의 제거가 의도적인지 확인해 주세요. 만약 메트릭 필터도 지원해야 한다면, 유니온 타입을 고려해 보시는 것이 좋겠습니다.
  2. undefined를 제거함으로써 filterType이 필수 속성이 되었습니다. 이것이 의도한 바인지 확인해 주세요.

다음 스크립트를 실행하여 GAMetricFilterType의 사용 여부를 확인해 주세요:

✅ Verification successful

이전 스크립트에서 typescript 파일 유형을 인식하지 못했습니다. 올바른 파일 유형을 사용하여 다시 시도하겠습니다.


filterType 속성 변경이 확인되었습니다

filterTypeGADimensionFilterType으로 변경된 것이 PR의 목적에 부합하며, GAMetricFilterType의 사용 여부를 확인한 결과 현재 코드베이스 내에서 사용되지 않는 것으로 보입니다. 필요하지 않다면 GAMetricFilterType의 정의도 제거하는 방안을 고려해보세요.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if GAMetricFilterType is still used in the codebase

# Test: Search for GAMetricFilterType usage
rg --type typescript 'GAMetricFilterType'

Length of output: 76


Script:

#!/bin/bash
# Description: Check if GAMetricFilterType is still used in the codebase

# Test: Search for GAMetricFilterType usage using the correct TypeScript file type
rg --type ts 'GAMetricFilterType'

Length of output: 165

src/QueryEditorGA4.tsx (3)

17-17: 타입 임포트가 올바르게 수정되었습니다.

'GAFilterExpression' 타입을 추가하여 타입 정의를 업데이트하셨습니다. 적절한 수정입니다.


125-127: onFiltersExpressionChange 함수의 인자 타입이 개선되었습니다.

함수의 인자를 일반적인 'any' 타입에서 'GAFilterExpression' 타입으로 변경하여 타입 안전성을 향상시켰습니다. 좋은 수정입니다.


14-14: 'Filter' 모듈의 임포트 경로를 확인해주세요.

'Filter' 모듈이 프로젝트 내에서 올바르게 임포트되고 있는지 확인해주세요. 상대 경로 또는 절대 경로를 사용할 때 경로가 정확한지 확인해야 합니다.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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

🧹 Outside diff range and nitpick comments (2)
src/Filter.tsx (2)

6-20: 컴포넌트 프롭스와 초기 설정이 개선되었습니다.

컴포넌트의 이름 변경과 새로운 프롭스 추가는 기능 확장을 잘 반영하고 있습니다. 표현식 타입의 도입으로 더 복잡한 필터링이 가능해졌습니다.

onDelete 프롭의 타입을 더 명확하게 하기 위해 다음과 같이 변경하는 것을 고려해보세요:

onDelete?: (() => void) | undefined;

이렇게 하면 onDelete가 함수이거나 undefined일 수 있다는 것을 더 명확히 표현할 수 있습니다.


50-109: 표현식 렌더링 로직이 잘 구조화되어 있습니다.

표현식 타입별로 분리된 렌더링 함수와 재귀적 접근 방식은 복잡한 중첩 표현식을 효과적으로 처리할 수 있게 해줍니다.

성능 최적화를 위해 다음 사항을 고려해보세요:

  1. 깊게 중첩된 구조의 경우 성능 문제가 발생할 수 있습니다. React.memo를 사용하여 불필요한 리렌더링을 방지할 수 있습니다:
const GAFilterExpressionComponent: React.FC<Props> = React.memo(({ expression, onChange, onDelete, selectedDimensions }) => {
  // ... existing code ...
});
  1. 큰 규모의 표현식 트리의 경우, 가상화 기법(예: react-window)을 사용하여 렌더링 성능을 개선할 수 있습니다.

이러한 최적화는 대규모 필터 표현식을 다룰 때 사용자 경험을 향상시킬 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 4cd9505 and 03f0d5a.

📒 Files selected for processing (1)
  • src/Filter.tsx (1 hunks)
🧰 Additional context used

Comment on lines +111 to +214
} as GAStringFilter
targetData.stringFilter = stringFilter
}
}
}
data[index].filter! = targetData
dimensionFilter.orGroup!.expressions = data
onChange({
...query, dimensionFilter
})
}

// const fieldNameChange = (value: SelectableValue<string>, index: number) => {
// let data = [...filterFields];
// const { query, onChange} = props;

// data[index].filedName = value.value || ''
// setFormFields(data);
//
// onChange({...query, metricFilter: data[index]})
// }

onChange({ filter: newFilter });
};

const renderFilterContent = () => {
switch (filter.filterType) {
case GADimensionFilterType.STRING:
return renderStringFilter(filter.stringFilter!);
case GADimensionFilterType.IN_LIST:
return renderInListFilter(filter.inListFilter!);
default:
return null;
}
};

return (
<VerticalGroup>
<Select
options={selectedDimensions}
value={filter.fieldName}
onChange={(option) => onChange({ filter: { ...filter, fieldName: option.value! } })}
placeholder="Select field"
/>
<Select
options={filterTypes}
value={filter.filterType}
onChange={handleFilterTypeChange}
/>
{renderFilterContent()}
</VerticalGroup>
);
};

const renderStringFilter = (stringFilter: GAStringFilter) => {
const matchTypes = Object.values(GAStringFilterMatchType).map(value => ({ label: value, value }));

return (
<VerticalGroup>
<Select
options={matchTypes}
value={stringFilter.matchType}
onChange={(option) => onChange({ filter: { ...expression.filter!, stringFilter: { ...stringFilter, matchType: option.value! } } })}
/>
<Input
value={stringFilter.value}
onChange={(e) => onChange({ filter: { ...expression.filter!, stringFilter: { ...stringFilter, value: e.currentTarget.value } } })}
placeholder="Value"
/>
<RadioButtonGroup
options={[
{ label: 'Case sensitive', value: true },
{ label: 'Case insensitive', value: false },
]}
value={stringFilter.caseSensitive}
onChange={(value) => onChange({ filter: { ...expression.filter!, stringFilter: { ...stringFilter, caseSensitive: value } } })}
/>
</VerticalGroup>
);
};

const renderInListFilter = (inListFilter: GAInListFilter = { values: [], caseSensitive: false }) => {
return (
<VerticalGroup>
<Input
value={inListFilter.values.join(', ')}
onChange={(e) => onChange({ filter: { ...expression.filter!, inListFilter: { ...inListFilter, values: e.currentTarget.value.split(',').map(v => v.trim()) } } })}
placeholder="Values (comma-separated)"
/>
<RadioButtonGroup
options={[
{ label: 'Case sensitive', value: true },
{ label: 'Case insensitive', value: false },
]}
value={inListFilter.caseSensitive}
onChange={(value) => onChange({ filter: { ...expression.filter!, inListFilter: { ...inListFilter, caseSensitive: value } } })}
/>
</VerticalGroup>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

필터 렌더링 로직 개선 제안

필터 타입별 렌더링 로직이 잘 구조화되어 있고, Grafana UI 컴포넌트를 일관되게 사용하고 있습니다. 하지만 코드 재사용성과 타입 안전성을 더욱 개선할 수 있습니다.

다음과 같은 개선 사항을 고려해보세요:

  1. 공통 로직을 별도의 함수로 추출하여 코드 중복을 줄입니다. 예를 들어, RadioButtonGroup을 렌더링하는 로직은 두 필터 타입에서 유사하게 사용됩니다.

  2. 타입 가드를 사용하여 타입 안전성을 높입니다:

function isStringFilter(filter: GAFilter): filter is GAFilter & { stringFilter: GAStringFilter } {
  return filter.filterType === GADimensionFilterType.STRING;
}

function isInListFilter(filter: GAFilter): filter is GAFilter & { inListFilter: GAInListFilter } {
  return filter.filterType === GADimensionFilterType.IN_LIST;
}

// 사용 예:
if (isStringFilter(filter)) {
  // filter.stringFilter는 여기서 타입 안전합니다.
}
  1. 필터 타입별 로직을 별도의 컴포넌트로 분리하여 코드의 모듈성을 높입니다.

이러한 변경으로 코드의 유지보수성과 확장성이 향상될 것입니다.

Comment on lines 216 to +233
<>
<HorizontalGroup>
<Field label="dimension">
<Select
options={props.query.selectedDimensions}
onChange={(value, action) => {
action.name = "dimensionChanged"
fieldNameChange(value, action, index)
}}
value={filter?.fieldName}
/>
</Field>
<Field label="filter type">

<Select
options={dimensionFilterType}
onChange={(value, action) => {
action.name = "filterTypeChanged"
fieldNameChange(value, action, index)
}}
value={filter?.filterType?.toString()}
/>
</Field>

{
filter?.filterType === GADimensionFilterType.STRING &&
<>
<Field label="match type">
<Select
options={stringFilterMatchType}
onChange={(value, action) => {
action.name = "matchTypeChanged"
fieldNameChange(value, action, index)
}}
value={filter.stringFilter?.matchType.toString()}
/>
</Field>

</>
}
{
filter?.filterType === GADimensionFilterType.STRING &&
<>
<Field label="value" invalid={filter.stringFilter?.value === ''} error={filter.stringFilter?.value === '' ? 'This input is required' : ''}>
<Input required onChange={(e) => {
filedValueChange(e.currentTarget.value, index)
}}
value={filter.stringFilter?.value}
></Input>
</Field>
</>
}
{
filter?.filterType === GADimensionFilterType.IN_LIST &&
<>
<Field label="values sperate by comma" invalid={filter.inListFilter?.values.join(',') === ''} error={filter.inListFilter?.values.join(',') === '' ? 'This input is required' : ''} >
<Input required onChange={(e) => {
filedValueChange(e.currentTarget.value, index)
}}
value={filter.inListFilter?.values.join(',')}
></Input>
</Field>
</>
}
{/* <Field label="filter type" description="filter type"> */}
<Button variant='secondary' icon='minus' onClick={() => removeFields(index)} ></Button>
<Button variant='secondary' icon='plus' onClick={addFields} ></Button>

{/* </Field> */}
</HorizontalGroup>

</>
)
})}
{
(Object.keys(dimensionFilter).length === 0 || dimensionFilter.orGroup?.expressions.length === 0) && <Button variant='secondary' icon='plus' onClick={addFields} ></Button>
}
</VerticalGroup>
{/* <Field label="filter type" description="filter type"> */}
{/* </Field> */}
</div>
</>
<FieldSet>
<HorizontalGroup>
<Select
options={expressionTypes}
value={Object.keys(expression).length === 0 ? 'none' : Object.keys(expression)[0]}
onChange={handleExpressionTypeChange}
/>
{renderExpressionContent()}
{onDelete && (
<Button variant="destructive" onClick={onDelete}>
Delete
</Button>
)}
</HorizontalGroup>
</FieldSet>
);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

메인 렌더 함수의 접근성 및 사용자 경험 개선 제안

메인 렌더 함수가 Grafana UI 컴포넌트를 적절히 사용하고 있으며, 삭제 버튼의 조건부 렌더링은 좋은 방식입니다.

접근성과 사용자 경험을 더욱 개선하기 위해 다음 사항을 고려해보세요:

  1. aria-label 속성을 추가하여 스크린 리더 사용자를 위한 접근성을 향상시킵니다:
<Select
  options={expressionTypes}
  value={Object.keys(expression).length === 0 ? 'none' : Object.keys(expression)[0]}
  onChange={handleExpressionTypeChange}
  aria-label="필터 표현식 타입 선택"
/>
  1. 삭제 버튼에 확인 대화상자를 추가하여 실수로 인한 삭제를 방지합니다:
<Button
  variant="destructive"
  onClick={() => {
    if (window.confirm('이 필터 표현식을 삭제하시겠습니까?')) {
      onDelete();
    }
  }}
>
  삭제
</Button>
  1. 툴팁을 추가하여 각 옵션에 대한 추가 정보를 제공합니다:
<Select
  options={expressionTypes.map(option => ({
    ...option,
    tooltip: `${option.label} 표현식을 생성합니다.`
  }))}
  // ... 기존 props
/>

이러한 개선사항들은 컴포넌트의 사용성과 접근성을 크게 향상시킬 것입니다.

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

🧹 Outside diff range and nitpick comments (1)
src/Filter.tsx (1)

7-12: Props 인터페이스에 JSDoc 문서화 추가 필요

각 prop의 목적과 사용법을 명확히 하기 위해 JSDoc 문서화를 추가하는 것이 좋습니다.

다음과 같이 문서화를 추가해보세요:

+/**
+ * GAFilterExpressionComponent의 Props 인터페이스
+ * @interface Props
+ */
 interface Props {
+  /** 현재 필터 표현식 */
   expression: GAFilterExpression;
+  /** 필터 표현식이 변경될 때 호출되는 콜백 */
   onChange: (expression: GAFilterExpression) => void;
+  /** 필터 삭제 시 호출되는 콜백 */
   onDelete?: () => void;
+  /** 선택 가능한 차원 목록 */
   selectedDimensions: Array<SelectableValue<string>>;
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 03f0d5a and abe35d3.

📒 Files selected for processing (1)
  • src/Filter.tsx (1 hunks)
🔇 Additional comments (2)
src/Filter.tsx (2)

120-139: 필터 타입 변경 처리가 잘 구현되었습니다

이전 리뷰에서 지적된 delete 연산자 사용을 undefined 할당으로 수정하여 성능 및 가독성이 개선되었습니다.


217-234: 메인 렌더 함수가 잘 구현되었습니다

컴포넌트의 구조가 명확하고, 조건부 렌더링이 적절하게 사용되었습니다.

Comment on lines +170 to +215
const renderStringFilter = (stringFilter: GAStringFilter) => {
const matchTypes = Object.values(GAStringFilterMatchType).map(value => ({ label: value, value }));

return (
<VerticalGroup>
<Select
options={matchTypes}
value={stringFilter.matchType}
onChange={(option) => onChange({ filter: { ...expression.filter!, stringFilter: { ...stringFilter, matchType: option.value! } } })}
/>
<Input
value={stringFilter.value}
onChange={(e) => onChange({ filter: { ...expression.filter!, stringFilter: { ...stringFilter, value: e.currentTarget.value } } })}
placeholder="Value"
/>
<RadioButtonGroup
options={[
{ label: 'Case sensitive', value: true },
{ label: 'Case insensitive', value: false },
]}
value={stringFilter.caseSensitive}
onChange={(value) => onChange({ filter: { ...expression.filter!, stringFilter: { ...stringFilter, caseSensitive: value } } })}
/>
</VerticalGroup>
);
};

const renderInListFilter = (inListFilter: GAInListFilter = { values: [], caseSensitive: false }) => {
return (
<VerticalGroup>
<Input
value={inListFilter.values.join(', ')}
onChange={(e) => onChange({ filter: { ...expression.filter!, inListFilter: { ...inListFilter, values: e.currentTarget.value.split(',').map(v => v.trim()) } } })}
placeholder="Values (comma-separated)"
/>
<RadioButtonGroup
options={[
{ label: 'Case sensitive', value: true },
{ label: 'Case insensitive', value: false },
]}
value={inListFilter.caseSensitive}
onChange={(value) => onChange({ filter: { ...expression.filter!, inListFilter: { ...inListFilter, caseSensitive: value } } })}
/>
</VerticalGroup>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

필터 렌더링 함수의 중복 코드 리팩토링 필요

renderStringFilterrenderInListFilter 함수에서 RadioButtonGroup 관련 코드가 중복되어 있습니다.

다음과 같이 공통 컴포넌트로 추출하는 것을 제안합니다:

const CaseSensitivityToggle: React.FC<{
  value: boolean;
  onChange: (value: boolean) => void;
}> = ({ value, onChange }) => (
  <RadioButtonGroup
    options={[
      { label: 'Case sensitive', value: true },
      { label: 'Case insensitive', value: false },
    ]}
    value={value}
    onChange={onChange}
  />
);

이를 각 필터 렌더링 함수에서 재사용할 수 있습니다.

Comment on lines +101 to +110
const renderNotExpression = () => {
return (
<GAFilterExpressionComponent
expression={expression.notExpression!}
selectedDimensions={selectedDimensions}
onChange={(newExpr) => onChange({ notExpression: newExpr })}
onDelete={() => onChange({})}
/>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

renderNotExpression 함수의 타입 안전성 개선 필요

expression.notExpression!에서 non-null assertion을 사용하고 있어 런타임 오류가 발생할 수 있습니다.

다음과 같이 타입 체크를 추가하는 것이 좋습니다:

 const renderNotExpression = () => {
+  if (!expression.notExpression) {
+    console.error('notExpression이 정의되지 않았습니다');
+    return null;
+  }
   return (
     <GAFilterExpressionComponent
-      expression={expression.notExpression!}
+      expression={expression.notExpression}
       selectedDimensions={selectedDimensions}
       onChange={(newExpr) => onChange({ notExpression: newExpr })}
       onDelete={() => onChange({})}
     />
   );
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const renderNotExpression = () => {
return (
<GAFilterExpressionComponent
expression={expression.notExpression!}
selectedDimensions={selectedDimensions}
onChange={(newExpr) => onChange({ notExpression: newExpr })}
onDelete={() => onChange({})}
/>
);
};
const renderNotExpression = () => {
if (!expression.notExpression) {
console.error('notExpression이 정의되지 않았습니다');
return null;
}
return (
<GAFilterExpressionComponent
expression={expression.notExpression}
selectedDimensions={selectedDimensions}
onChange={(newExpr) => onChange({ notExpression: newExpr })}
onDelete={() => onChange({})}
/>
);
};

Comment on lines +23 to +49
const handleExpressionTypeChange = (option: SelectableValue<string>) => {
let newExpression: GAFilterExpression;
switch (option.value) {
case 'none':
newExpression = {};
break;
case 'andGroup':
case 'orGroup':
newExpression = { [option.value]: { expressions: [] } };
break;
case 'notExpression':
newExpression = { notExpression: {} };
break;
case GADimensionFilterType.IN_LIST:
if (targetData.inListFilter !== undefined) {
targetData.inListFilter.values = value.split(',')
data[index].filter = targetData
}
case 'filter':
newExpression = {
filter: {
fieldName: '',
filterType: GADimensionFilterType.STRING,
stringFilter: { matchType: GAStringFilterMatchType.EXACT, value: '', caseSensitive: false }
}
};
break;
default:
return;
}
dimensionFilter.orGroup!.expressions = data
onChange({
...query, dimensionFilter
})
}
const fieldNameChange = (value: SelectableValue<string>, action: ActionMeta, index: number) => {
let data = [...dimensionFilter.orGroup!.expressions];
let targetData = data[index].filter!
const { query, onChange } = props;
if (value.value !== undefined) {
switch (action.name) {
case "dimensionChanged":
targetData.fieldName = value.value
onChange(newExpression);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

handleExpressionTypeChange 함수의 오류 처리 개선 필요

switch 문의 default 케이스가 비어있어 예상치 못한 타입이 들어올 경우 적절한 처리가 되지 않습니다.

다음과 같이 개선해보세요:

 const handleExpressionTypeChange = (option: SelectableValue<string>) => {
+  if (!option.value) {
+    console.error('옵션 값이 없습니다');
+    return;
+  }
   let newExpression: GAFilterExpression;
   switch (option.value) {
     // ... existing cases ...
     default:
-      return;
+      console.error(`지원하지 않는 표현식 타입입니다: ${option.value}`);
+      return;
   }
   onChange(newExpression);
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleExpressionTypeChange = (option: SelectableValue<string>) => {
let newExpression: GAFilterExpression;
switch (option.value) {
case 'none':
newExpression = {};
break;
case 'andGroup':
case 'orGroup':
newExpression = { [option.value]: { expressions: [] } };
break;
case 'notExpression':
newExpression = { notExpression: {} };
break;
case GADimensionFilterType.IN_LIST:
if (targetData.inListFilter !== undefined) {
targetData.inListFilter.values = value.split(',')
data[index].filter = targetData
}
case 'filter':
newExpression = {
filter: {
fieldName: '',
filterType: GADimensionFilterType.STRING,
stringFilter: { matchType: GAStringFilterMatchType.EXACT, value: '', caseSensitive: false }
}
};
break;
default:
return;
}
dimensionFilter.orGroup!.expressions = data
onChange({
...query, dimensionFilter
})
}
const fieldNameChange = (value: SelectableValue<string>, action: ActionMeta, index: number) => {
let data = [...dimensionFilter.orGroup!.expressions];
let targetData = data[index].filter!
const { query, onChange } = props;
if (value.value !== undefined) {
switch (action.name) {
case "dimensionChanged":
targetData.fieldName = value.value
onChange(newExpression);
};
const handleExpressionTypeChange = (option: SelectableValue<string>) => {
if (!option.value) {
console.error('옵션 값이 없습니다');
return;
}
let newExpression: GAFilterExpression;
switch (option.value) {
case 'none':
newExpression = {};
break;
case 'andGroup':
case 'orGroup':
newExpression = { [option.value]: { expressions: [] } };
break;
case 'notExpression':
newExpression = { notExpression: {} };
break;
case 'filter':
newExpression = {
filter: {
fieldName: '',
filterType: GADimensionFilterType.STRING,
stringFilter: { matchType: GAStringFilterMatchType.EXACT, value: '', caseSensitive: false }
}
};
break;
default:
console.error(`지원하지 않는 표현식 타입입니다: ${option.value}`);
return;
}
onChange(newExpression);
};

@lcc3108 lcc3108 merged commit 906bad4 into master Oct 29, 2024
7 checks passed
@lcc3108 lcc3108 deleted the feature/frontend/support-full-dimensions-filter branch October 29, 2024 09:58
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