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

code smells #1264

Merged
merged 7 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions starsky/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ RUN rm /app/global.json

RUN \
if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \
echo $TARGETPLATFORM ; \
echo "$TARGETPLATFORM" ; \
dotnet restore --runtime linux-x64 starsky.csproj ; \
elif [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
dotnet restore --runtime linux-arm64 starsky.csproj ; \
Expand Down Expand Up @@ -85,9 +85,9 @@ RUN \
DEMO_SEED_CLI_PATH="/app/starskydemoseedcli/starskydemoseedcli.csproj" ;\
DEPS_FOLDER="/app/dependencies" ;\
TEMP_FOLDER="/app/temp" ;\
mkdir -p $DEPS_FOLDER ;\
mkdir -p $TEMP_FOLDER ;\
dotnet run --project $DEMO_SEED_CLI_PATH --configuration Release -- --dependencies $DEPS_FOLDER --tempfolder $TEMP_FOLDER -h -v ;\
mkdir -p "$DEPS_FOLDER" ;\
mkdir -p "$TEMP_FOLDER" ;\
dotnet run --project "$DEMO_SEED_CLI_PATH" --configuration Release -- --dependencies "$DEPS_FOLDER" --tempfolder "$TEMP_FOLDER" -h -v ;\
fi

# no alpine build since there is no support for multi-arch
Expand All @@ -101,7 +101,7 @@ WORKDIR /app
RUN if [ "$TEST" = "true" ]; then \
mkdir -p "/testresults" ;\
if [ "$BUILDPLATFORM" = "$TARGETPLATFORM" ]; then \
echo $TEST $BUILDPLATFORM $TARGETPLATFORM ; \
echo "$TEST" "$BUILDPLATFORM" "$TARGETPLATFORM" ; \
dotnet test -c release --results-directory /testresults --logger "trx;LogFileName=test_results.trx" --collect:"XPlat Code Coverage" --settings build.vstest.runsettings starskytest/starskytest.csproj ; \
fi ;\
touch "/testresults/test.enabled" ;\
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -20,11 +21,11 @@ public class CheckForUpdates : ICheckForUpdates
{
internal const string GithubApi = "https://api.github.com/repos/qdraw/starsky/releases";

private readonly AppSettings _appSettings;
private readonly IMemoryCache _cache;
private readonly AppSettings? _appSettings;
private readonly IMemoryCache? _cache;
private readonly IHttpClientHelper _httpClientHelper;

public CheckForUpdates(IHttpClientHelper httpClientHelper, AppSettings appSettings, IMemoryCache cache)
public CheckForUpdates(IHttpClientHelper httpClientHelper, AppSettings? appSettings, IMemoryCache? cache)
{
_httpClientHelper = httpClientHelper;
_appSettings = appSettings;
Expand All @@ -47,31 +48,36 @@ public async Task<KeyValuePair<UpdateStatus, string>> IsUpdateNeeded(string curr
? _appSettings.AppVersion : currentVersion;

// The CLI programs uses no cache
if( _cache == null || _appSettings?.AddMemoryCache == false)
if ( _cache == null || _appSettings?.AddMemoryCache == false )
{
return Parse(await QueryIsUpdateNeededAsync(),currentVersion);
}

if ( _cache.TryGetValue(QueryCheckForUpdatesCacheName, out var cacheResult) )
if ( _cache.TryGetValue(QueryCheckForUpdatesCacheName,
out var cacheResult) )
{
return Parse(( List<ReleaseModel> ) cacheResult, currentVersion);
}

cacheResult = await QueryIsUpdateNeededAsync();

_cache.Set(QueryCheckForUpdatesCacheName, cacheResult,
new TimeSpan(48,0,0));
new TimeSpan(48,0,0));

return Parse(( List<ReleaseModel> ) cacheResult,currentVersion);
return Parse(( List<ReleaseModel>? ) cacheResult,currentVersion);
}

internal async Task<List<ReleaseModel>> QueryIsUpdateNeededAsync()
internal async Task<List<ReleaseModel>?> QueryIsUpdateNeededAsync()
{
// argument check is done in QueryIsUpdateNeeded
var (key, value) = await _httpClientHelper.ReadString(GithubApi);
return !key ? new List<ReleaseModel>() : JsonSerializer.Deserialize<List<ReleaseModel>>(value, new JsonSerializerOptions());
}

internal static KeyValuePair<UpdateStatus, string> Parse(IEnumerable<ReleaseModel> releaseModelList, string currentVersion )
internal static KeyValuePair<UpdateStatus, string> Parse(IEnumerable<ReleaseModel>? releaseModelList, string currentVersion )
{
var orderedReleaseModelList = releaseModelList.OrderByDescending(p => p.TagName);
var tagName = orderedReleaseModelList.FirstOrDefault(p => !p.Draft && !p.PreRelease)?.TagName;
var orderedReleaseModelList = releaseModelList?.OrderByDescending(p => p.TagName);
var tagName = orderedReleaseModelList?.FirstOrDefault(p => p is { Draft: false, PreRelease: false })?.TagName;
if ( string.IsNullOrWhiteSpace(tagName) ||
!tagName.StartsWith('v') )
{
Expand Down
39 changes: 26 additions & 13 deletions starsky/starsky.feature.search/ViewModels/SearchViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ public string ParseDefaultOption(string defaultQuery)
return returnQueryBuilder.ToString();
}

private (string defaultQuery, StringBuilder returnQueryBuilder) ParseQuotedValues(string defaultQuery, StringBuilder returnQueryBuilder)
private (string defaultQuery, StringBuilder returnQueryBuilder)
ParseQuotedValues(string defaultQuery, StringBuilder returnQueryBuilder)
{
// Get Quoted values
// (["'])(\\?.)*?\1
Expand Down Expand Up @@ -535,7 +536,7 @@ public static SearchViewModel NarrowSearch(SearchViewModel model)
return model;
}

private static SearchViewModel PropertySearchStringType(
internal static SearchViewModel PropertySearchStringType(
SearchViewModel model,
PropertyInfo property, string searchForQuery,
SearchForOptionType searchType)
Expand All @@ -546,27 +547,37 @@ private static SearchViewModel PropertySearchStringType(
model.FileIndexItems = model.FileIndexItems!.Where(
p => p.GetType().GetProperty(property.Name)?.Name == property.Name
&& ! // not
p.GetType().GetProperty(property.Name)!.GetValue(p, null)!
.ToString()!.ToLowerInvariant().Contains(searchForQuery)
p.GetType().GetProperty(property.Name)!.GetValue(p, null)?
.ToString()?.ToLowerInvariant().Contains(searchForQuery) == true
).ToList();
break;
default:
model.FileIndexItems = model.FileIndexItems!
model.FileIndexItems = model.FileIndexItems?
.Where(p => p.GetType().GetProperty(property.Name)?.Name == property.Name
&& p.GetType().GetProperty(property.Name)!.GetValue(p, null)!
.ToString()!.ToLowerInvariant().Contains(searchForQuery)
&& p.GetType().GetProperty(property.Name)!.GetValue(p, null)?
.ToString()?.ToLowerInvariant().Contains(searchForQuery) == true
).ToList();
break;
}

return model;
}

private static SearchViewModel PropertySearchBoolType(
SearchViewModel model,
PropertyInfo property, bool boolIsValue)
internal static SearchViewModel PropertySearchBoolType(
SearchViewModel? model,
PropertyInfo? property, bool boolIsValue)
{
model.FileIndexItems = model.FileIndexItems!
if ( model == null )
{
return new SearchViewModel();
}

if ( property == null)
{
return model;
}

model.FileIndexItems = model.FileIndexItems?
.Where(p => p.GetType().GetProperty(property.Name)?.Name == property.Name
&& (bool?) p.GetType().GetProperty(property.Name)?.GetValue(p, null) == boolIsValue
).ToList();
Expand Down Expand Up @@ -653,12 +664,14 @@ internal static SearchViewModel PropertySearch(SearchViewModel model,
return PropertySearchStringType(model, property, searchForQuery, searchType);
}

if ( property.PropertyType == typeof(bool) && bool.TryParse(searchForQuery, out var boolIsValue))
if ( (property.PropertyType == typeof(bool) || property.PropertyType == typeof(bool?)) &&
bool.TryParse(searchForQuery, out var boolIsValue))
{
return PropertySearchBoolType(model, property, boolIsValue);
}

if ( property.PropertyType == typeof(ExtensionRolesHelper.ImageFormat) && Enum.TryParse<ExtensionRolesHelper.ImageFormat>(
if ( property.PropertyType == typeof(ExtensionRolesHelper.ImageFormat) &&
Enum.TryParse<ExtensionRolesHelper.ImageFormat>(
searchForQuery.ToLowerInvariant(), out var castImageFormat) )
{
return PropertySearchImageFormatType(model, property, castImageFormat, searchType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void BuilderDb(string? foundationDatabaseName = "")
if ( _logger != null && _appSettings.IsVerbose() )
{
_logger.LogInformation($"Database connection: {_appSettings.DatabaseConnection}");
_logger?.LogInformation($"Application Insights Database tracking is {IsDatabaseTrackingEnabled()}" );
_logger.LogInformation($"Application Insights Database tracking is {IsDatabaseTrackingEnabled()}" );
}

#if ENABLE_DEFAULT_DATABASE
Expand Down
3 changes: 2 additions & 1 deletion starsky/starsky.foundation.database/Models/FileIndexItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ public static List<string> FileIndexPropList()
var fileIndexPropList = new List<string>();
// only for types String in FileIndexItem()

foreach (var propertyInfo in new FileIndexItem().GetType().GetProperties())
var allProperties = new FileIndexItem().GetType().GetProperties();
foreach (var propertyInfo in allProperties)
{
if ((
propertyInfo.PropertyType == typeof(bool) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private static FileIndexItem GetDataNullNameSpaceTypes(IXmpMeta xmp, FileIndexIt
!string.IsNullOrEmpty(property.Value) &&
string.IsNullOrEmpty(property.Namespace) )
{
StringBuilder tagsStringBuilder = new StringBuilder();
var tagsStringBuilder = new StringBuilder();
tagsStringBuilder.Append(item.Tags);
tagsStringBuilder.Append(", ");
tagsStringBuilder.Append(property.Value);
Expand Down
4 changes: 2 additions & 2 deletions starsky/starsky/clientapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"start": "vite --port 3000",
"build": "tsc -p tsconfig.prod.json && vite build",
"spell": "cspell --config cspell.json \"src/**/*.{ts,tsx,js,md}\" --exclude build",
"lint": "eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 800",
"lint:fix": "eslint --fix . --ext ts,tsx --report-unused-disable-directives --max-warnings 800",
"lint": "eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 810",
"lint:fix": "eslint --fix . --ext ts,tsx --report-unused-disable-directives --max-warnings 810",
"format": "prettier --write './**/*.{js,jsx,ts,tsx,css,md,json}'",
"test": "jest --watch",
"test:ci": "jest --ci --coverage --silent",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ import { memo } from "react";
import { IFileIndexItem } from "../../../interfaces/IFileIndexItem";

interface IDetailViewInfoMakeModelApertureProps {
fileIndexItem: IFileIndexItem;
fileIndexItem: Readonly<IFileIndexItem>;
}

function ShowISOIfExistComponent(fileIndexItemInside: IFileIndexItem) {
function ShowISOIfExistComponent(
fileIndexItemInside: Readonly<IFileIndexItem>
) {
return (
<>
{fileIndexItemInside.isoSpeed !== 0 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as PanAndZoomImage from "./pan-and-zoom-image";

describe("FileHashImage", () => {
it("renders", () => {
render(<FileHashImage isError={false} fileHash={""} />);
render(<FileHashImage fileHash={""} />);
});

beforeEach(() => {
Expand Down Expand Up @@ -53,7 +53,6 @@ describe("FileHashImage", () => {
// need to await here
const component = await render(
<FileHashImage
isError={false}
fileHash="hash"
orientation={Orientation.Horizontal}
/>
Expand Down Expand Up @@ -96,7 +95,6 @@ describe("FileHashImage", () => {

const component = render(
<FileHashImage
isError={false}
fileHash="hash"
orientation={Orientation.Horizontal}
/>
Expand Down Expand Up @@ -128,7 +126,6 @@ describe("FileHashImage", () => {

const component = render(
<FileHashImage
isError={false}
fileHash="hash"
orientation={Orientation.Horizontal}
/>
Expand Down Expand Up @@ -168,7 +165,6 @@ describe("FileHashImage", () => {

const component = render(
<FileHashImage
isError={false}
fileHash="hash"
orientation={Orientation.Horizontal}
id={"fallbackPath"}
Expand Down Expand Up @@ -216,7 +212,6 @@ describe("FileHashImage", () => {
const onWheelCallbackSpy = jest.fn();
const component = render(
<FileHashImage
isError={false}
fileHash="hash"
id="/test.jpg"
orientation={Orientation.Horizontal}
Expand Down Expand Up @@ -260,7 +255,6 @@ describe("FileHashImage", () => {

const component = render(
<FileHashImage
isError={false}
fileHash="hash"
id="/test.jpg"
orientation={Orientation.Horizontal}
Expand Down Expand Up @@ -310,7 +304,6 @@ describe("FileHashImage", () => {

const component = render(
<FileHashImage
isError={false}
fileHash="hash"
id="/test.jpg"
orientation={Orientation.Horizontal}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import PanAndZoomImage from "./pan-and-zoom-image";

export interface IFileHashImageProps {
setError?: React.Dispatch<React.SetStateAction<boolean>>;
isError: boolean;
setIsLoading?: React.Dispatch<React.SetStateAction<boolean>>;
fileHash: string;
orientation?: Orientation;
tags?: string;
id?: string; // filepath to know when image is changed
onWheelCallback?(z: number): void;
onResetCallback?(): void;
Expand Down
44 changes: 21 additions & 23 deletions starsky/starsky/clientapp/src/components/atoms/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,33 +66,31 @@ export default function Modal({

if (modal.current) {
return ReactDOM.createPortal(
<>
<div
onClick={(event) => ifModalOpenHandleExit(event, handleExit)}
data-test={dataTest}
className={`modal-bg ${
isOpen ? ` ${ModalOpenClassName} ` + className : ""
}`}
>
<div
onClick={(event) => ifModalOpenHandleExit(event, handleExit)}
data-test={dataTest}
className={`modal-bg ${
isOpen ? ` ${ModalOpenClassName} ` + className : ""
}`}
className={`modal-content ${isOpen ? " modal-content--show" : ""}`}
>
<div
className={`modal-content ${isOpen ? " modal-content--show" : ""}`}
>
<div className="modal-close-bar">
<button
className={`modal-exit-button ${
isOpen ? " modal-exit-button--showing" : ""
}`}
ref={exitButton}
data-test="modal-exit-button"
onClick={handleExit}
>
{MessageCloseDialog}
</button>
</div>
{children}
<div className="modal-close-bar">
<button
className={`modal-exit-button ${
isOpen ? " modal-exit-button--showing" : ""
}`}
ref={exitButton}
data-test="modal-exit-button"
onClick={handleExit}
>
{MessageCloseDialog}
</button>
</div>
{children}
</div>
</>,
</div>,
modal.current
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface ISwitchButtonProps {
"data-test"?: string;
}

function SwitchButton(props: ISwitchButtonProps) {
function SwitchButton(props: Readonly<ISwitchButtonProps>) {
const [random, setRandom] = useState(0);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,12 @@ const SearchPagination: React.FunctionComponent<IRelativeLink> = memo(
}

return (
<>
<div className="relativelink" data-test="search-pagination">
<h4 className="nextprev">
{prev()}
{next()}
</h4>
</div>
</>
<div className="relativelink" data-test="search-pagination">
<h4 className="nextprev">
{prev()}
{next()}
</h4>
</div>
);
}
);
Expand Down
Loading
Loading