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.

Sign up for GitHub

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

fix: OtelBin sonarCloud issues fix #173

Merged
merged 4 commits into from
Nov 5, 2023
Merged

fix: OtelBin sonarCloud issues fix #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0c0555
fix: OtelBin sonarCloud issues fix
Nov 4, 2023
1f2f0a4
fix: sonarCloud- Convert the conditional to a boolean
Nov 4, 2023
48d80e2
fix: sonarCloud- reduce code smell score
Nov 4, 2023
1fee446
enhance: minor errorConsole namings update
Nov 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions packages/otelbin/src/components/monaco-editor/Editor.tsx
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bripkens better now?

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

import { useCallback, useEffect, useMemo, useState } from "react";
import React from "react";
import type { IError } from "./ErrorConsole";
import ErrorConsole from "./ErrorConsole";
import type { IError } from "./ValidationErrorConsole";
import ValidationErrorConsole from "./ValidationErrorConsole";
import EditorTopBar from "../EditorTopBar";
import { useEditorRef, useEditorDidMount, useMonacoRef, useViewMode } from "~/contexts/EditorContext";
import MonacoEditor, { loader, type OnChange } from "@monaco-editor/react";
Expand Down Expand Up @@ -59,15 +59,16 @@ export default function Editor({ locked, setLocked }: { locked: boolean; setLock
[getLink]
);

const errors = useMemo((): IError => {
const totalValidationErrors = useMemo((): IError => {
if (editorRef && monacoRef) {
return validateOtelCollectorConfigurationAndSetMarkers(currentConfig, editorRef, monacoRef);
} else {
return {};
}
}, [currentConfig, editorRef, monacoRef]);

const isValidConfig = errors.jsYamlError == null && (errors.ajvErrors?.length ?? 0) === 0;
const isValidConfig =
totalValidationErrors.jsYamlError == null && (totalValidationErrors.ajvErrors?.length ?? 0) === 0;

const handleEditorChange: OnChange = (value) => {
setCurrentConfig(value || "");
Expand Down Expand Up @@ -167,7 +168,7 @@ export default function Editor({ locked, setLocked }: { locked: boolean; setLock
</AutoSizer>
)}
</div>
{viewMode !== "pipeline" && <ErrorConsole errors={errors} font={firaCode} />}
{viewMode !== "pipeline" && <ValidationErrorConsole errors={totalValidationErrors} font={firaCode} />}
{viewMode == "both" && <ResizeBar onWidthChange={onWidthChange} />}
</div>
<div className="z-0 min-h-full w-full shrink grow relative">
Expand Down
48 changes: 29 additions & 19 deletions ...components/monaco-editor/ErrorConsole.tsx → .../monaco-editor/ValidationErrorConsole.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface IError {
customWarnings?: string[];
}

export default function ErrorConsole({ errors, font }: { errors?: IError; font: NextFont }) {
export default function ValidationErrorConsole({ errors, font }: { errors?: IError; font: NextFont }) {
const serverSideValidationResult = useServerSideValidation();

const errorCount =
Expand All @@ -37,10 +37,10 @@ export default function ErrorConsole({ errors, font }: { errors?: IError; font:
const [isOpenErrorConsole, setIsOpenErrorConsole] = useState(false);

useEffect(() => {
if (errorCount === 0) {
if (errorCount === 0 && warningsCount === 0) {
setIsOpenErrorConsole(false);
}
}, [errorCount]);
}, [errorCount, warningsCount]);

return (
<div
Expand All @@ -50,13 +50,13 @@ export default function ErrorConsole({ errors, font }: { errors?: IError; font:
>
<div className="flex flex-col h-full">
<div className="flex items-center">
<ErrorCount
<ErrorAndWarningCounter
errorsCount={errorCount}
warningsCount={0}
isOpen={isOpenErrorConsole}
setOpen={setIsOpenErrorConsole}
/>
<ErrorCount
<ErrorAndWarningCounter
isWarning
errorsCount={0}
warningsCount={warningsCount}
Expand All @@ -69,10 +69,10 @@ export default function ErrorConsole({ errors, font }: { errors?: IError; font:
{errors?.customWarnings &&
errors.customWarnings?.length > 0 &&
errors.customWarnings.map((warning: string, index: number) => {
return <Error key={index} customWarnings={warning} font={font} />;
return <ErrorMessage key={index} customWarnings={warning} font={font} />;
})}
{serverSideValidationResult.result?.error && (
<Error
<ErrorMessage
serverSideError={
serverSideValidationResult.result?.message + " - " + serverSideValidationResult.result?.error
}
Expand All @@ -82,30 +82,30 @@ export default function ErrorConsole({ errors, font }: { errors?: IError; font:
{errors?.ajvErrors &&
errors.ajvErrors?.length > 0 &&
errors.ajvErrors.map((error: IAjvError, index: number) => {
return <Error key={index} error={error} font={font} />;
return <ErrorMessage key={index} ajvError={error} font={font} />;
})}
{errors?.customErrors &&
errors.customErrors?.length > 0 &&
errors.customErrors.map((error: string, index: number) => {
return <Error key={index} customErrors={error} font={font} />;
return <ErrorMessage key={index} customErrors={error} font={font} />;
})}
{errors?.jsYamlError?.mark?.line && <Error jsYamlError={errors && errors.jsYamlError} font={font} />}
{errors?.jsYamlError?.mark?.line && <ErrorMessage jsYamlError={errors && errors.jsYamlError} font={font} />}
</div>
)}
</div>
</div>
);
}

export function Error({
error,
export function ErrorMessage({
ajvError,
jsYamlError,
serverSideError,
customErrors,
customWarnings,
font,
}: {
error?: IAjvError;
ajvError?: IAjvError;
jsYamlError?: IJsYamlError;
serverSideError?: string;
customErrors?: string;
Expand All @@ -121,9 +121,9 @@ export function Error({
<p>{`${customWarnings}`}</p>
</div>
)}
{error && (
{ajvError && (
<div className={`${font.className} ${errorsStyle}`}>
<p>{`${error.message}`}</p>
<p>{`${ajvError.message}`}</p>
</div>
)}
{customErrors && (
Expand All @@ -149,7 +149,7 @@ export function Error({
);
}

export function ErrorCount({
export function ErrorAndWarningCounter({
errorsCount,
warningsCount,
isOpen,
Expand All @@ -167,7 +167,12 @@ export function ErrorCount({
{isWarning ? (
<div
onClick={() => {
if (errorsCount > 0 || warningsCount || 0 > 0) {
if (errorsCount || warningsCount) {
setOpen(!isOpen);
}
}}
onKeyDown={(e) => {
if (e.key === "Enter") {
setOpen(!isOpen);
}
}}
Expand All @@ -179,13 +184,18 @@ export function ErrorCount({
<p className="text-xs font-medium">{`${warningsCount} ${
(warningsCount || 0) === 1 ? `Warning` : `Warnings`
}`}</p>
{((warningsCount || 0) > 0 || errorsCount > 0) && <ChevronDown width={12} color="#64748b" />}
{Boolean(warningsCount) || Boolean(errorsCount) ? <ChevronDown width={12} color="#64748b" /> : <></>}
</div>
</div>
) : (
<div
onClick={() => {
if (errorsCount > 0 || warningsCount || 0 > 0) {
if (errorsCount || warningsCount) {
setOpen(!isOpen);
}
}}
onKeyDown={(e) => {
if (e.key === "Enter") {
setOpen(!isOpen);
}
}}
Expand Down
2 changes: 1 addition & 1 deletion packages/otelbin/src/components/monaco-editor/otelCollectorConfigValidation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import { schema } from "./JSONSchema";
import type { IAjvError, IError, IJsYamlError } from "./ErrorConsole";
import type { IAjvError, IError, IJsYamlError } from "./ValidationErrorConsole";
import JsYaml from "js-yaml";
import Ajv from "ajv";
import type { ErrorObject } from "ajv";
Expand Down
6 changes: 5 additions & 1 deletion packages/otelbin/src/components/react-flow/FlowClick.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ export interface IData {
id: string;
}

export function FlowClick(event: React.MouseEvent, data: IData, editorRef: EditorRefType | null) {
export function FlowClick(
event: React.MouseEvent | React.KeyboardEvent<HTMLDivElement>,
data: IData,
editorRef: EditorRefType | null
) {
event.stopPropagation();
const config = editorRef?.current?.getModel()?.getValue() || "";
const docElements = getParsedValue(config);
Expand Down
7 changes: 6 additions & 1 deletion packages/otelbin/src/components/react-flow/node-types/Node.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const Node = ({
borderBottom: `1px solid ${hovered ? "#6D737D" : "#40454E"}`,
};

function handleClickNode(event: React.MouseEvent<HTMLDivElement, MouseEvent>) {
function handleClickNode(event: React.MouseEvent<HTMLDivElement, MouseEvent> | React.KeyboardEvent<HTMLDivElement>) {
FlowClick(event, data, editorRef);
}

Expand Down Expand Up @@ -86,6 +86,11 @@ const Node = ({
style={customNodeStyles}
className="cursor-pointer flex-col"
onClick={handleClickNode}
onKeyDown={(e) => {
if (e.key === "Space") {
handleClickNode(e);
}
}}
onMouseEnter={() => setHovered(true)}
onMouseLeave={() => setHovered(false)}
>
Expand Down
122 changes: 58 additions & 64 deletions packages/otelbin/src/components/react-flow/useNodes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,86 +50,80 @@ const createNode = (pipelineName: string, parentNode: IPipeline, height: number,
const processorLength = (processors?.length ?? 0) * 200 + 260;
return { x: processorLength, y: positionY !== undefined ? positionY : parentHeight / 2 };
};
const processors = parentNode.processors;
const receivers = parentNode.receivers;
const exporters = parentNode.exporters;
keyTraces.forEach((traceItem) => {
switch (traceItem) {
case "processors":
Array.isArray(processors) &&
processors.length > 0 &&
processors.map((processor, index) => {
const id = `${pipelineName}-Processor-processorNode-${processor}`;

nodesToAdd.push({
id: id,
parentNode: pipelineName,
extent: "parent",
type: "processorsNode",
position: processorPosition(index, height || 100, processors),
data: {
label: processor,
parentNode: pipelineName,
type: "processors",
height: childNodesHeight,
id: id,
},
draggable: false,
});
});
break;
case "receivers":
Array.isArray(receivers) &&
receivers.length > 0 &&
receivers.map((receiver, index) => {
const isConnector = connectors && Object.keys(connectors).includes(receiver);

keyTraces.map((traceItem) => {
if (traceItem === "processors") {
const processors = parentNode.processors;
Array.isArray(processors) &&
processors.length > 0 &&
processors.map((processor, index) => {
const id = `${pipelineName}-Processor-processorNode-${processor}`;
const id = `${pipelineName}-Receiver-receiverNode-${receiver}`;

nodesToAdd.push({
id: id,
parentNode: pipelineName,
extent: "parent",
type: "processorsNode",
position: processorPosition(index, height || 100, processors),
data: {
label: processor,
parentNode: pipelineName,
type: "processors",
height: childNodesHeight,
nodesToAdd.push({
id: id,
},
draggable: false,
parentNode: pipelineName,
extent: "parent",
type: "receiversNode",
position: receiverPosition(index, height || 100, receivers),
data: {
label: receiver,
parentNode: pipelineName,
type: isConnector ? "connectors/receivers" : "receivers",
height: childNodesHeight,
id: id,
},
draggable: false,
});
});
});
}
if (traceItem === "receivers") {
const receivers = parentNode.receivers;
Array.isArray(receivers) &&
receivers.length > 0 &&
receivers.map((receiver, index) => {
let isConnector = false;
if (connectors && Object.keys(connectors).includes(receiver)) {
isConnector = true;
}
const id = `${pipelineName}-Receiver-receiverNode-${receiver}`;

break;
case "exporters":
exporters?.map((exporter, index) => {
const isConnector = connectors && Object.keys(connectors).includes(exporter);
const id = `${pipelineName}-exporter-exporterNode-${exporter}`;
nodesToAdd.push({
id: id,
parentNode: pipelineName,
extent: "parent",
type: "receiversNode",
position: receiverPosition(index, height || 100, receivers),
type: "exportersNode",
position: exporterPosition(index, height || 100, exporters, processors ?? []),
data: {
label: receiver,
label: exporter,
parentNode: pipelineName,
type: isConnector ? "connectors/receivers" : "receivers",
type: isConnector ? "connectors/exporters" : "exporters",
height: childNodesHeight,
id: id,
},
draggable: false,
});
});
}
if (traceItem === "exporters") {
const exporters = parentNode.exporters;
const processors = parentNode.processors;
exporters?.map((exporter, index) => {
let isConnector = false;
if (connectors && Object.keys(connectors).includes(exporter)) {
isConnector = true;
}

const id = `${pipelineName}-exporter-exporterNode-${exporter}`;
nodesToAdd.push({
id: id,
parentNode: pipelineName,
extent: "parent",
type: "exportersNode",
position: exporterPosition(index, height || 100, exporters, processors ?? []),
data: {
label: exporter,
parentNode: pipelineName,
type: isConnector ? "connectors/exporters" : "exporters",
height: childNodesHeight,
id: id,
},
draggable: false,
});
});
break;
}
});
return nodesToAdd;
Expand Down
Loading