From 1434516c20838363afe370e401d6b06652a3757f Mon Sep 17 00:00:00 2001 From: Dion Date: Sat, 21 Oct 2023 11:21:09 +0200 Subject: [PATCH 1/5] code smells --- starsky/Dockerfile | 10 +- .../UpdateCheck/Services/CheckForUpdates.cs | 28 ++-- .../ViewModels/SearchViewModel.cs | 10 +- .../Helpers/SetupDatebaseTypes.cs | 2 +- .../Models/FileIndexItem.cs | 3 +- .../ReadMetaHelpers/ReadMetaXmp.cs | 2 +- starsky/starsky/clientapp/package.json | 4 +- .../detailview-info-make-model-aperture.tsx | 6 +- .../atoms/file-hash-image/file-hash-image.tsx | 2 - .../src/components/atoms/modal/modal.tsx | 44 +++---- .../atoms/switch-button/switch-button.tsx | 2 +- .../search-pagination/search-pagination.tsx | 14 +- .../detailview-info-datetime.tsx | 4 +- .../detailview-info-location.tsx | 4 +- .../organisms/menu-default/menu-default.tsx | 26 ++-- .../modal-download/modal-download.tsx | 4 +- .../organisms/modal-publish/modal-publish.tsx | 4 +- .../preferences-password.tsx | 121 +++++++++--------- .../src/containers/detailview/detailview.tsx | 1 - .../clientapp/src/containers/search.tsx | 2 +- .../clientapp/src/containers/trash.tsx | 2 +- .../src/shared/update-relative-object.ts | 8 +- 22 files changed, 147 insertions(+), 156 deletions(-) diff --git a/starsky/Dockerfile b/starsky/Dockerfile index 21af2d4225..915e47a5cc 100644 --- a/starsky/Dockerfile +++ b/starsky/Dockerfile @@ -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 ; \ @@ -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 @@ -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" ;\ diff --git a/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs b/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs index 7b0e6d06b8..1f410fd714 100644 --- a/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs +++ b/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs @@ -1,3 +1,4 @@ +#nullable enable using System; using System.Collections.Generic; using System.Linq; @@ -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; @@ -47,31 +48,36 @@ public async Task> 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 ) cacheResult, currentVersion); + } cacheResult = await QueryIsUpdateNeededAsync(); _cache.Set(QueryCheckForUpdatesCacheName, cacheResult, - new TimeSpan(48,0,0)); + new TimeSpan(48,0,0)); - return Parse(( List ) cacheResult,currentVersion); + return Parse(( List? ) cacheResult,currentVersion); } - internal async Task> QueryIsUpdateNeededAsync() + internal async Task?> QueryIsUpdateNeededAsync() { // argument check is done in QueryIsUpdateNeeded var (key, value) = await _httpClientHelper.ReadString(GithubApi); return !key ? new List() : JsonSerializer.Deserialize>(value, new JsonSerializerOptions()); } - internal static KeyValuePair Parse(IEnumerable releaseModelList, string currentVersion ) + internal static KeyValuePair Parse(IEnumerable? 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.Draft && !p.PreRelease)?.TagName; if ( string.IsNullOrWhiteSpace(tagName) || !tagName.StartsWith('v') ) { diff --git a/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs b/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs index 3198b6756d..a698ffa367 100644 --- a/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs +++ b/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs @@ -546,15 +546,15 @@ 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; } diff --git a/starsky/starsky.foundation.database/Helpers/SetupDatebaseTypes.cs b/starsky/starsky.foundation.database/Helpers/SetupDatebaseTypes.cs index bc00db9e10..8b9cb0a152 100644 --- a/starsky/starsky.foundation.database/Helpers/SetupDatebaseTypes.cs +++ b/starsky/starsky.foundation.database/Helpers/SetupDatebaseTypes.cs @@ -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 diff --git a/starsky/starsky.foundation.database/Models/FileIndexItem.cs b/starsky/starsky.foundation.database/Models/FileIndexItem.cs index 29b4490096..0f8adb6da5 100644 --- a/starsky/starsky.foundation.database/Models/FileIndexItem.cs +++ b/starsky/starsky.foundation.database/Models/FileIndexItem.cs @@ -481,7 +481,8 @@ public static List FileIndexPropList() var fileIndexPropList = new List(); // 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) || diff --git a/starsky/starsky.foundation.readmeta/ReadMetaHelpers/ReadMetaXmp.cs b/starsky/starsky.foundation.readmeta/ReadMetaHelpers/ReadMetaXmp.cs index 66666d9f81..464dbbd862 100644 --- a/starsky/starsky.foundation.readmeta/ReadMetaHelpers/ReadMetaXmp.cs +++ b/starsky/starsky.foundation.readmeta/ReadMetaHelpers/ReadMetaXmp.cs @@ -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); diff --git a/starsky/starsky/clientapp/package.json b/starsky/starsky/clientapp/package.json index fd7b55684a..904a2ef4db 100644 --- a/starsky/starsky/clientapp/package.json +++ b/starsky/starsky/clientapp/package.json @@ -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", diff --git a/starsky/starsky/clientapp/src/components/atoms/detailview-info-make-model-aperture/detailview-info-make-model-aperture.tsx b/starsky/starsky/clientapp/src/components/atoms/detailview-info-make-model-aperture/detailview-info-make-model-aperture.tsx index ef4c807a67..b20ede29eb 100644 --- a/starsky/starsky/clientapp/src/components/atoms/detailview-info-make-model-aperture/detailview-info-make-model-aperture.tsx +++ b/starsky/starsky/clientapp/src/components/atoms/detailview-info-make-model-aperture/detailview-info-make-model-aperture.tsx @@ -2,10 +2,12 @@ import { memo } from "react"; import { IFileIndexItem } from "../../../interfaces/IFileIndexItem"; interface IDetailViewInfoMakeModelApertureProps { - fileIndexItem: IFileIndexItem; + fileIndexItem: Readonly; } -function ShowISOIfExistComponent(fileIndexItemInside: IFileIndexItem) { +function ShowISOIfExistComponent( + fileIndexItemInside: Readonly +) { return ( <> {fileIndexItemInside.isoSpeed !== 0 ? ( diff --git a/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.tsx b/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.tsx index 13d995fdbd..9e82e1ca7d 100644 --- a/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.tsx +++ b/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.tsx @@ -7,11 +7,9 @@ import PanAndZoomImage from "./pan-and-zoom-image"; export interface IFileHashImageProps { setError?: React.Dispatch>; - isError: boolean; setIsLoading?: React.Dispatch>; fileHash: string; orientation?: Orientation; - tags?: string; id?: string; // filepath to know when image is changed onWheelCallback?(z: number): void; onResetCallback?(): void; diff --git a/starsky/starsky/clientapp/src/components/atoms/modal/modal.tsx b/starsky/starsky/clientapp/src/components/atoms/modal/modal.tsx index 2399ae79a2..0801fe9a90 100644 --- a/starsky/starsky/clientapp/src/components/atoms/modal/modal.tsx +++ b/starsky/starsky/clientapp/src/components/atoms/modal/modal.tsx @@ -66,33 +66,31 @@ export default function Modal({ if (modal.current) { return ReactDOM.createPortal( - <> +
ifModalOpenHandleExit(event, handleExit)} + data-test={dataTest} + className={`modal-bg ${ + isOpen ? ` ${ModalOpenClassName} ` + className : "" + }`} + >
ifModalOpenHandleExit(event, handleExit)} - data-test={dataTest} - className={`modal-bg ${ - isOpen ? ` ${ModalOpenClassName} ` + className : "" - }`} + className={`modal-content ${isOpen ? " modal-content--show" : ""}`} > -
-
- -
- {children} +
+
+ {children}
- , +
, modal.current ); } diff --git a/starsky/starsky/clientapp/src/components/atoms/switch-button/switch-button.tsx b/starsky/starsky/clientapp/src/components/atoms/switch-button/switch-button.tsx index b32395f9af..b5ae0bb321 100644 --- a/starsky/starsky/clientapp/src/components/atoms/switch-button/switch-button.tsx +++ b/starsky/starsky/clientapp/src/components/atoms/switch-button/switch-button.tsx @@ -10,7 +10,7 @@ export interface ISwitchButtonProps { "data-test"?: string; } -function SwitchButton(props: ISwitchButtonProps) { +function SwitchButton(props: Readonly) { const [random, setRandom] = useState(0); useEffect(() => { diff --git a/starsky/starsky/clientapp/src/components/molecules/search-pagination/search-pagination.tsx b/starsky/starsky/clientapp/src/components/molecules/search-pagination/search-pagination.tsx index e213881635..d93d8b2765 100644 --- a/starsky/starsky/clientapp/src/components/molecules/search-pagination/search-pagination.tsx +++ b/starsky/starsky/clientapp/src/components/molecules/search-pagination/search-pagination.tsx @@ -93,14 +93,12 @@ const SearchPagination: React.FunctionComponent = memo( } return ( - <> -
-

- {prev()} - {next()} -

-
- +
+

+ {prev()} + {next()} +

+
); } ); diff --git a/starsky/starsky/clientapp/src/components/organisms/detailview-info-datetime/detailview-info-datetime.tsx b/starsky/starsky/clientapp/src/components/organisms/detailview-info-datetime/detailview-info-datetime.tsx index 7bec2d6f5e..30d6cd76b4 100644 --- a/starsky/starsky/clientapp/src/components/organisms/detailview-info-datetime/detailview-info-datetime.tsx +++ b/starsky/starsky/clientapp/src/components/organisms/detailview-info-datetime/detailview-info-datetime.tsx @@ -23,7 +23,7 @@ const DetailViewInfoDateTime: React.FunctionComponent {/* dateTime when the image is created */} - {isModalDatetimeOpen ? ( + {modalDatetimeOpen ? ( {/* loation when the image is created */} - {isLocationOpen ? ( + {locationOpen ? ( = () => { const [hamburgerMenu, setHamburgerMenu] = useState(false); return ( - <> -
-
- +
+
+ - - setHamburgerMenu(!hamburgerMenu)} /> - -
-
- + + setHamburgerMenu(!hamburgerMenu)} /> + +
+
); }; diff --git a/starsky/starsky/clientapp/src/components/organisms/modal-download/modal-download.tsx b/starsky/starsky/clientapp/src/components/organisms/modal-download/modal-download.tsx index d7e5fadf4e..df5b903867 100644 --- a/starsky/starsky/clientapp/src/components/organisms/modal-download/modal-download.tsx +++ b/starsky/starsky/clientapp/src/components/organisms/modal-download/modal-download.tsx @@ -199,9 +199,7 @@ const ModalDownload: React.FunctionComponent = (props) => { ) : null} {isProcessing === ProcessingState.server ? ( - <> -
- +
) : null} {isProcessing === ProcessingState.fail diff --git a/starsky/starsky/clientapp/src/components/organisms/modal-publish/modal-publish.tsx b/starsky/starsky/clientapp/src/components/organisms/modal-publish/modal-publish.tsx index 0454c44c30..078e2fd148 100644 --- a/starsky/starsky/clientapp/src/components/organisms/modal-publish/modal-publish.tsx +++ b/starsky/starsky/clientapp/src/components/organisms/modal-publish/modal-publish.tsx @@ -218,9 +218,7 @@ const ModalPublish: React.FunctionComponent = (props) => { ) : null} {isProcessing === ProcessingState.server ? ( - <> -
- +
) : null} {isProcessing === ProcessingState.fail ? ( diff --git a/starsky/starsky/clientapp/src/components/organisms/preferences-password/preferences-password.tsx b/starsky/starsky/clientapp/src/components/organisms/preferences-password/preferences-password.tsx index 182e09d4ee..109cf247bf 100644 --- a/starsky/starsky/clientapp/src/components/organisms/preferences-password/preferences-password.tsx +++ b/starsky/starsky/clientapp/src/components/organisms/preferences-password/preferences-password.tsx @@ -95,72 +95,67 @@ const PreferencesPassword: React.FunctionComponent = () => { } return ( - <> -
{ - e.preventDefault(); - setError(null); - if (validateChangePassword()) { - await changeSecret(); - } - }} - > -
{MessageChangePassword}
-
- - setUserCurrentPassword(e.target.value)} - /> + { + e.preventDefault(); + setError(null); + if (validateChangePassword()) { + await changeSecret(); + } + }} + > +
{MessageChangePassword}
+
+ + setUserCurrentPassword(e.target.value)} + /> - - setUserChangedPassword(e.target.value)} - /> + + setUserChangedPassword(e.target.value)} + /> - - setUserChangedConfirmPassword(e.target.value)} - /> - {error && ( -
- {error} -
- )} + + setUserChangedConfirmPassword(e.target.value)} + /> + {error && ( +
+ {error} +
+ )} - - {loading ? "Loading..." : MessageChangePassword} - -
- - + + {loading ? "Loading..." : MessageChangePassword} + +
+ ); }; diff --git a/starsky/starsky/clientapp/src/containers/detailview/detailview.tsx b/starsky/starsky/clientapp/src/containers/detailview/detailview.tsx index 9dae8bc60c..51f66ffd93 100644 --- a/starsky/starsky/clientapp/src/containers/detailview/detailview.tsx +++ b/starsky/starsky/clientapp/src/containers/detailview/detailview.tsx @@ -245,7 +245,6 @@ const DetailView: React.FC = () => { ) { // Content const settings = useGlobalSettings(); const language = new Language(settings.language); diff --git a/starsky/starsky/clientapp/src/containers/trash.tsx b/starsky/starsky/clientapp/src/containers/trash.tsx index 515d305376..d6a003675a 100644 --- a/starsky/starsky/clientapp/src/containers/trash.tsx +++ b/starsky/starsky/clientapp/src/containers/trash.tsx @@ -13,7 +13,7 @@ import { IArchiveProps } from "../interfaces/IArchiveProps"; import { Language } from "../shared/language"; import { URLPath } from "../shared/url-path"; -function Trash(archive: IArchiveProps) { +function Trash(archive: Readonly) { // Content const settings = useGlobalSettings(); const language = new Language(settings.language); diff --git a/starsky/starsky/clientapp/src/shared/update-relative-object.ts b/starsky/starsky/clientapp/src/shared/update-relative-object.ts index 63ccfa17d8..5d3b67eab9 100644 --- a/starsky/starsky/clientapp/src/shared/update-relative-object.ts +++ b/starsky/starsky/clientapp/src/shared/update-relative-object.ts @@ -15,7 +15,7 @@ export class UpdateRelativeObject { ): Promise { return new Promise((resolve, rejects) => { if (state.subPath === "/" || !isSearchQuery) { - rejects(); + rejects(new Error("no subpath or query")); return; } @@ -28,15 +28,15 @@ export class UpdateRelativeObject { ) .then((result) => { if (result.statusCode !== 200) { - rejects(); + rejects(new Error("status code not 200")); return; } setRelativeObjects(result.data); resolve(result.data); }) .catch((err) => { - console.log(err); - rejects(); + console.error(err); + rejects(err); }); }); } From 9b8641c536f7919e6550bf42596202ffdf282821 Mon Sep 17 00:00:00 2001 From: Dion Date: Sat, 21 Oct 2023 11:27:13 +0200 Subject: [PATCH 2/5] remove unused prop --- .../atoms/file-hash-image/file-hash-image.spec.tsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.spec.tsx b/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.spec.tsx index 94ba3416de..a38819d399 100644 --- a/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.spec.tsx +++ b/starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.spec.tsx @@ -12,7 +12,7 @@ import * as PanAndZoomImage from "./pan-and-zoom-image"; describe("FileHashImage", () => { it("renders", () => { - render(); + render(); }); beforeEach(() => { @@ -53,7 +53,6 @@ describe("FileHashImage", () => { // need to await here const component = await render( @@ -96,7 +95,6 @@ describe("FileHashImage", () => { const component = render( @@ -128,7 +126,6 @@ describe("FileHashImage", () => { const component = render( @@ -168,7 +165,6 @@ describe("FileHashImage", () => { const component = render( { const onWheelCallbackSpy = jest.fn(); const component = render( { const component = render( { const component = render( Date: Tue, 31 Oct 2023 11:36:12 +0100 Subject: [PATCH 3/5] add tests --- .../UpdateCheck/Services/CheckForUpdates.cs | 4 +- .../ViewModels/SearchViewModel.cs | 29 ++- .../starskytest/Models/SearchViewModelTest.cs | 225 ++++++++++++++++++ .../Helpers/CheckForUpdatesHelperTest.cs | 48 ++++ 4 files changed, 296 insertions(+), 10 deletions(-) diff --git a/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs b/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs index 1f410fd714..222ba78780 100644 --- a/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs +++ b/starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs @@ -73,11 +73,11 @@ public async Task> IsUpdateNeeded(string curr var (key, value) = await _httpClientHelper.ReadString(GithubApi); return !key ? new List() : JsonSerializer.Deserialize>(value, new JsonSerializerOptions()); } - + internal static KeyValuePair Parse(IEnumerable? releaseModelList, string currentVersion ) { var orderedReleaseModelList = releaseModelList?.OrderByDescending(p => p.TagName); - var tagName = orderedReleaseModelList?.FirstOrDefault(p => !p.Draft && !p.PreRelease)?.TagName; + var tagName = orderedReleaseModelList?.FirstOrDefault(p => p is { Draft: false, PreRelease: false })?.TagName; if ( string.IsNullOrWhiteSpace(tagName) || !tagName.StartsWith('v') ) { diff --git a/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs b/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs index a698ffa367..b3b4384404 100644 --- a/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs +++ b/starsky/starsky.feature.search/ViewModels/SearchViewModel.cs @@ -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 @@ -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) @@ -562,11 +563,21 @@ private static SearchViewModel PropertySearchStringType( 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(); @@ -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( + if ( property.PropertyType == typeof(ExtensionRolesHelper.ImageFormat) && + Enum.TryParse( searchForQuery.ToLowerInvariant(), out var castImageFormat) ) { return PropertySearchImageFormatType(model, property, castImageFormat, searchType); diff --git a/starsky/starskytest/Models/SearchViewModelTest.cs b/starsky/starskytest/Models/SearchViewModelTest.cs index 06df0f204a..e1810aba50 100644 --- a/starsky/starskytest/Models/SearchViewModelTest.cs +++ b/starsky/starskytest/Models/SearchViewModelTest.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using Microsoft.VisualStudio.TestTools.UnitTesting; using starsky.feature.search.ViewModels; using starsky.foundation.database.Models; @@ -33,5 +35,228 @@ public void PropertySearchTest() Assert.AreEqual(0, search.CollectionsCount); } + + [TestMethod] + public void PropertySearchStringType_DefaultCase_NullConditions1() + { + // Arrange + var model = new SearchViewModel + { + FileIndexItems = null + }; + + var property = typeof(FileIndexItem).GetProperty("NotFound"); + Assert.IsNull(property); + + const string searchForQuery = "file"; + var searchType = SearchViewModel.SearchForOptionType.Equal; + + // Act + var result = SearchViewModel.PropertySearchStringType(model, property, searchForQuery, searchType); + + // Assert + Assert.IsNotNull(result); + Assert.IsNull(result.FileIndexItems); + } + + [TestMethod] + public void PropertySearchStringType_DefaultCase_NullConditions2() + { + // Arrange + var model = new SearchViewModel + { + FileIndexItems = null + }; + + var property = typeof(FileIndexItem).GetProperty(nameof(FileIndexItem.Tags)); + const string searchForQuery = "file"; + var searchType = SearchViewModel.SearchForOptionType.Equal; + + // Act + var result = SearchViewModel.PropertySearchStringType(model, property, searchForQuery, searchType); + + // Assert + Assert.IsNotNull(result); + Assert.IsNull(result.FileIndexItems); + } + + [TestMethod] + public void PropertySearchStringType_DefaultCase_Found_Null() + { + // Arrange + var model = new SearchViewModel + { + FileIndexItems = new List{new FileIndexItem("test"){LocationCity = null}} + }; + + var property = typeof(FileIndexItem).GetProperty(nameof(FileIndexItem.LocationCity)); + const string searchForQuery = "file"; + var searchType = SearchViewModel.SearchForOptionType.Equal; + + // Act + var result = SearchViewModel.PropertySearchStringType(model, property, searchForQuery, searchType); + + // Assert + Assert.IsNotNull(result); + Assert.AreEqual(0,result.FileIndexItems?.Count); + } + + [TestMethod] + public void PropertySearchStringType_DefaultCase_Found_HappyFlow() + { + // Arrange + var model = new SearchViewModel + { + FileIndexItems = new List{new FileIndexItem("test"){LocationCity = "test"}} + }; + + var property = typeof(FileIndexItem).GetProperty(nameof(FileIndexItem.LocationCity)); + const string searchForQuery = "test"; + var searchType = SearchViewModel.SearchForOptionType.Equal; + + // Act + var result = SearchViewModel.PropertySearchStringType(model, property, searchForQuery, searchType); + + // Assert + Assert.IsNotNull(result); + Assert.AreEqual(1,result.FileIndexItems?.Count); + } + + [TestMethod] + public void PropertySearchBoolType_FiltersItemsByBoolProperty() + { + // Arrange + var model = new SearchViewModel(); + model.FileIndexItems = new List + { + new FileIndexItem { IsDirectory = true }, + new FileIndexItem { IsDirectory = false }, + new FileIndexItem { IsDirectory = true }, + }; + var property = typeof(FileIndexItem).GetProperty("IsDirectory"); + var boolIsValue = true; + + // Act + var result = SearchViewModel.PropertySearchBoolType(model, property, boolIsValue); + + // Assert + Assert.AreEqual(2, result.FileIndexItems?.Count); + Assert.IsTrue(result.FileIndexItems?.All(item => item.IsDirectory == true)); + } + + [TestMethod] + public void PropertySearchBoolType_WithNullModel_ReturnsNullModel() + { + // Arrange + SearchViewModel model = null; + var property = typeof(FileIndexItem).GetProperty("IsDirectory"); + const bool boolIsValue = true; + + // Act + var result = SearchViewModel.PropertySearchBoolType(model, property, boolIsValue); + + // Assert + Assert.IsNotNull(result); + } + + [TestMethod] + public void PropertySearchBoolType_WithNullFileIndexItems_ReturnsNullFileIndexItems() + { + // Arrange + var model = new SearchViewModel + { + FileIndexItems = null, + }; + var property = typeof(FileIndexItem).GetProperty("IsDirectory"); + var boolIsValue = true; + + // Act + var result = SearchViewModel.PropertySearchBoolType(model, property, boolIsValue); + + // Assert + Assert.IsNull(result.FileIndexItems); + } + + [TestMethod] + public void PropertySearchBoolType_WithEmptyFileIndexItems_ReturnsEmptyFileIndexItems() + { + // Arrange + var model = new SearchViewModel + { + FileIndexItems = new List(), + }; + var property = typeof(FileIndexItem).GetProperty("IsDirectory"); + var boolIsValue = true; + + // Act + var result = SearchViewModel.PropertySearchBoolType(model, property, boolIsValue); + + // Assert + Assert.IsNotNull(result.FileIndexItems); + Assert.AreEqual(0, result.FileIndexItems.Count); + } + + [TestMethod] + public void PropertySearchBoolType_WithInvalidProperty_ReturnsOriginalModel() + { + // Arrange + var model = new SearchViewModel(); + model.FileIndexItems = new List + { + new FileIndexItem { IsDirectory = true }, + }; + var property = typeof(FileIndexItem).GetProperty("NonExistentProperty"); + var boolIsValue = true; + + // Act + var result = SearchViewModel.PropertySearchBoolType(model, property, boolIsValue); + + // Assert + Assert.AreEqual(model, result); + } + + [TestMethod] + public void PropertySearch_WithBoolPropertyAndValidBoolValue_ReturnsFilteredModel() + { + // Arrange + var model = new SearchViewModel { FileIndexItems = new List + { + new FileIndexItem { IsDirectory = true }, + new FileIndexItem { IsDirectory = false }, + new FileIndexItem { IsDirectory = true }, + } + }; + var property = typeof(FileIndexItem).GetProperty("IsDirectory"); + const string searchForQuery = "true"; + const SearchViewModel.SearchForOptionType searchType = SearchViewModel.SearchForOptionType.Equal; + + // Act + var result = SearchViewModel.PropertySearch(model, property, searchForQuery, searchType); + + // Assert + Assert.AreEqual(2, result.FileIndexItems?.Count); + Assert.IsTrue(result.FileIndexItems?.All(item => item.IsDirectory == true)); + } + + [TestMethod] + public void PropertySearch_WithBoolPropertyAndInvalidBoolValue_ReturnsOriginalModel() + { + // Arrange + var model = new SearchViewModel(); + model.FileIndexItems = new List + { + new FileIndexItem { IsDirectory = true }, + new FileIndexItem { IsDirectory = false }, + }; + var property = typeof(FileIndexItem).GetProperty("IsDirectory"); + var searchForQuery = "invalid_bool_value"; // An invalid boolean string + var searchType = SearchViewModel.SearchForOptionType.Equal; // You can set this as needed + + // Act + var result = SearchViewModel.PropertySearch(model, property, searchForQuery, searchType); + + // Assert + CollectionAssert.AreEqual(model.FileIndexItems, result.FileIndexItems); + } } } diff --git a/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs b/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs index 7e6767316c..b77a808e06 100644 --- a/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs +++ b/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs @@ -292,5 +292,53 @@ public async Task IsUpdateNeeded_CacheIsNull() Assert.IsNotNull(results); Assert.AreEqual(UpdateStatus.NoReleasesFound,results.Key); } + + [TestMethod] + public void Parse_WithNullReleaseModelList_ReturnsNoReleasesFound() + { + // Arrange + IEnumerable releaseModelList = null; + string currentVersion = "1.0.0"; // Provide a valid version + + // Act + var result = CheckForUpdates.Parse(releaseModelList, currentVersion); + + // Assert + Assert.AreEqual(UpdateStatus.NoReleasesFound, result.Key); + Assert.AreEqual(string.Empty, result.Value); + } + + [TestMethod] + public void Parse_WithNullOrderedReleaseModelList_ReturnsNoReleasesFound() + { + // Arrange + IEnumerable releaseModelList = new List(); // An empty list + string currentVersion = "1.0.0"; // Provide a valid version + + // Act + var result = CheckForUpdates.Parse(releaseModelList, currentVersion); + + // Assert + Assert.AreEqual(UpdateStatus.NoReleasesFound, result.Key); + Assert.AreEqual(string.Empty, result.Value); + } + + [TestMethod] + public void Parse_WithNullFirstReleaseModel_ReturnsNoReleasesFound() + { + // Arrange + IEnumerable releaseModelList = new List + { + // List with no valid releases (Draft and PreRelease) + }; + string currentVersion = "1.0.0"; // Provide a valid version + + // Act + var result = CheckForUpdates.Parse(releaseModelList, currentVersion); + + // Assert + Assert.AreEqual(UpdateStatus.NoReleasesFound, result.Key); + Assert.AreEqual(string.Empty, result.Value); + } } } From 5b61162c9105ee557223dd2e21fe0c27d65ddb51 Mon Sep 17 00:00:00 2001 From: Dion Date: Tue, 31 Oct 2023 11:52:59 +0100 Subject: [PATCH 4/5] small changes --- .../Helpers/CheckForUpdatesHelperTest.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs b/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs index b77a808e06..6333297139 100644 --- a/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs +++ b/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs @@ -297,11 +297,10 @@ public async Task IsUpdateNeeded_CacheIsNull() public void Parse_WithNullReleaseModelList_ReturnsNoReleasesFound() { // Arrange - IEnumerable releaseModelList = null; - string currentVersion = "1.0.0"; // Provide a valid version + const string currentVersion = "1.0.0"; // Provide a valid version // Act - var result = CheckForUpdates.Parse(releaseModelList, currentVersion); + var result = CheckForUpdates.Parse(null, currentVersion); // Assert Assert.AreEqual(UpdateStatus.NoReleasesFound, result.Key); @@ -313,7 +312,7 @@ public void Parse_WithNullOrderedReleaseModelList_ReturnsNoReleasesFound() { // Arrange IEnumerable releaseModelList = new List(); // An empty list - string currentVersion = "1.0.0"; // Provide a valid version + const string currentVersion = "1.0.0"; // Provide a valid version // Act var result = CheckForUpdates.Parse(releaseModelList, currentVersion); @@ -331,7 +330,7 @@ public void Parse_WithNullFirstReleaseModel_ReturnsNoReleasesFound() { // List with no valid releases (Draft and PreRelease) }; - string currentVersion = "1.0.0"; // Provide a valid version + const string currentVersion = "1.0.0"; // Provide a valid version // Act var result = CheckForUpdates.Parse(releaseModelList, currentVersion); From 46cd26a89dff71db4eb404846ba46fbf01e9804f Mon Sep 17 00:00:00 2001 From: Dion Date: Tue, 31 Oct 2023 11:57:28 +0100 Subject: [PATCH 5/5] add test --- .../Helpers/CheckForUpdatesHelperTest.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs b/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs index 6333297139..e016d1b2f9 100644 --- a/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs +++ b/starsky/starskytest/starsky.feature.health/Helpers/CheckForUpdatesHelperTest.cs @@ -339,5 +339,27 @@ public void Parse_WithNullFirstReleaseModel_ReturnsNoReleasesFound() Assert.AreEqual(UpdateStatus.NoReleasesFound, result.Key); Assert.AreEqual(string.Empty, result.Value); } + + + [TestMethod] + public void Parse_WithNullFirstReleaseModel_Returns_EmptyReleaseModel() + { + // Arrange + IEnumerable releaseModelList = new List + { + new ReleaseModel + { + TagName = null + } + }; + const string currentVersion = "1.0.0"; // Provide a valid version + + // Act + var result = CheckForUpdates.Parse(releaseModelList, currentVersion); + + // Assert + Assert.AreEqual(UpdateStatus.NoReleasesFound, result.Key); + Assert.AreEqual(string.Empty, result.Value); + } } }