Skip to content

Commit 2b1ce0f

Browse files
EtschbeijerEtschbeijer
authored
Merge pull request #1185 from nfdi4plants/epic/feature/InstallBasicHandlingForDatasets
Remove delete and rename button from datasets and standard children
2 parents 6a6f89a + b92c1bc commit 2b1ce0f

8 files changed

Lines changed: 269 additions & 58 deletions

File tree

‎src/Components/src/Primitive/Popover/Popover.stories.tsx‎

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -212,42 +212,42 @@ export const NonModal: Story = {
212212
},
213213
};
214214

215-
//export const ModalFocus: Story = {
216-
// render: () => <ModalPopoverExample />,
217-
// play: async ({ canvasElement }) => {
218-
// const canvas = within(canvasElement);
219-
// const trigger = canvas.getByRole("button", { name: /open modal/i });
220-
// const afterTarget = canvas.getByRole("button", { name: /after target/i });
221-
222-
// trigger.focus();
223-
// expect(trigger).toHaveFocus();
224-
225-
// await userEvent.click(trigger);
226-
227-
// const dialog = await screen.findByTestId("popover_content_modal");
228-
// const firstAction = screen.getByTestId(TESTID_MODAL_PRIMARY_ACTION);
229-
// const closeButton = screen.getByRole("button", { name: /close modal/i });
230-
231-
// await waitFor(() => {
232-
// expect(dialog.contains(document.activeElement)).toBe(true);
233-
// });
234-
235-
// await userEvent.tab();
236-
// expect(dialog.contains(document.activeElement)).toBe(true);
237-
238-
// await userEvent.tab();
239-
// expect(dialog.contains(document.activeElement)).toBe(true);
240-
// expect(document.activeElement === firstAction || document.activeElement === closeButton).toBe(true);
241-
// expect(afterTarget).not.toHaveFocus();
242-
243-
// await userEvent.click(closeButton);
244-
245-
// await waitFor(() =>
246-
// expect(screen.queryByRole("dialog", { name: /modal popover/i })).not.toBeInTheDocument(),
247-
// );
248-
// await waitFor(() => expect(trigger).toHaveFocus());
249-
// },
250-
//};
215+
export const ModalFocus: Story = {
216+
render: () => <ModalPopoverExample />,
217+
play: async ({ canvasElement }) => {
218+
const canvas = within(canvasElement);
219+
const trigger = canvas.getByRole("button", { name: /open modal/i });
220+
const afterTarget = canvas.getByRole("button", { name: /after target/i });
221+
222+
trigger.focus();
223+
expect(trigger).toHaveFocus();
224+
225+
await userEvent.click(trigger);
226+
227+
const dialog = await screen.findByTestId("popover_content_modal");
228+
const firstAction = screen.getByTestId(TESTID_MODAL_PRIMARY_ACTION);
229+
const closeButton = screen.getByRole("button", { name: /close modal/i });
230+
231+
await waitFor(() => {
232+
expect(dialog.contains(document.activeElement)).toBe(true);
233+
});
234+
235+
await userEvent.tab();
236+
expect(dialog.contains(document.activeElement)).toBe(true);
237+
238+
await userEvent.tab();
239+
expect(dialog.contains(document.activeElement)).toBe(true);
240+
expect(document.activeElement === firstAction || document.activeElement === closeButton).toBe(true);
241+
expect(afterTarget).not.toHaveFocus();
242+
243+
await userEvent.click(closeButton);
244+
245+
await waitFor(() =>
246+
expect(screen.queryByRole("dialog", { name: /modal popover/i })).not.toBeInTheDocument(),
247+
);
248+
await waitFor(() => expect(trigger).toHaveFocus());
249+
},
250+
};
251251

252252
export const DuplicateHeading: Story = {
253253
render: () => <DuplicateHeadingPopoverExample />,

‎src/Shared/PathHelpers.fs‎

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ module ArcEntityPathRules =
148148
| CanonicalDataMapFileTarget of zone: AddZone * identifier: string * normalizedRelativePath: string
149149
| GenericTarget of normalizedRelativePath: string
150150

151+
type private StructuralArcPath =
152+
| AddZoneRoot
153+
| EntityFolder
154+
| ProtectedEntityChildFolder
155+
| OtherPath
156+
151157
let private protectedDeleteTargetNames = [ ".gitattributes"; ".gitkeep"; "readme.md" ]
152158
let private protectedRenameRootFolderNames = [ "notes" ]
153159
let private disallowedGenericPathSegments = [ ".git" ]
@@ -172,6 +178,19 @@ module ArcEntityPathRules =
172178
| AddZone.Workflows -> ARCtrl.ArcPathHelper.WorkflowFileName
173179
| AddZone.Runs -> ARCtrl.ArcPathHelper.RunFileName
174180

181+
let private nativeEntityChildFolderNames =
182+
function
183+
| AddZone.Studies -> [
184+
ARCtrl.ArcPathHelper.StudiesProtocolsFolderName
185+
ARCtrl.ArcPathHelper.StudiesResourcesFolderName
186+
]
187+
| AddZone.Assays -> [
188+
ARCtrl.ArcPathHelper.AssayDatasetFolderName
189+
ARCtrl.ArcPathHelper.AssayProtocolsFolderName
190+
]
191+
| AddZone.Workflows
192+
| AddZone.Runs -> []
193+
175194
let private tryParseZone (segment: string) =
176195
if PathHelpers.pathsEqual segment ARCtrl.ArcPathHelper.StudiesFolderName then
177196
Some AddZone.Studies
@@ -184,6 +203,31 @@ module ArcEntityPathRules =
184203
else
185204
None
186205

206+
let private isAddZoneSegment segment = (tryParseZone segment).IsSome
207+
208+
let private classifyStructuralArcPath =
209+
function
210+
| [| zoneSegment |] when isAddZoneSegment zoneSegment -> StructuralArcPath.AddZoneRoot
211+
| [| zoneSegment; _ |] when isAddZoneSegment zoneSegment -> StructuralArcPath.EntityFolder
212+
| [| zoneSegment; _; childFolderName |] ->
213+
match tryParseZone zoneSegment with
214+
| Some zone when PathHelpers.pathMatchesAny (nativeEntityChildFolderNames zone) childFolderName ->
215+
StructuralArcPath.ProtectedEntityChildFolder
216+
| _ -> StructuralArcPath.OtherPath
217+
| _ -> StructuralArcPath.OtherPath
218+
219+
let private blocksGenericFileSystemTarget =
220+
function
221+
| StructuralArcPath.AddZoneRoot
222+
| StructuralArcPath.EntityFolder
223+
| StructuralArcPath.ProtectedEntityChildFolder -> true
224+
| StructuralArcPath.OtherPath -> false
225+
226+
let private isProtectedTarget normalizedRelativePath =
227+
function
228+
| StructuralArcPath.ProtectedEntityChildFolder -> true
229+
| _ -> PathHelpers.isProtectedDeleteTarget protectedDeleteTargetNames normalizedRelativePath
230+
187231
let private tryParseCanonicalArcFileTargetFromSegments (segments: string[]) =
188232
if segments.Length = 0 then
189233
None
@@ -214,10 +258,7 @@ module ArcEntityPathRules =
214258

215259
let private containsDisallowedGenericPathSegment (segments: string[]) =
216260
segments
217-
|> Array.exists (fun segment ->
218-
disallowedGenericPathSegments
219-
|> List.exists (fun blocked -> PathHelpers.pathsEqual segment blocked)
220-
)
261+
|> Array.exists (PathHelpers.pathMatchesAny disallowedGenericPathSegments)
221262

222263
/// Parses canonical ARC file targets from the tail of a path and supports absolute paths.
223264
let tryParseCanonicalArcFileTarget (path: string) =
@@ -228,14 +269,14 @@ module ArcEntityPathRules =
228269

229270
let classifyDeleteTarget (relativePath: string) =
230271
let normalizedRelativePath = normalizeRelativePath relativePath
272+
let segments = normalizedRelativePath |> splitPathSegments
273+
let structuralPath = classifyStructuralArcPath segments
231274

232275
if String.IsNullOrWhiteSpace normalizedRelativePath then
233276
DeletePathClassification.DisallowedTarget normalizedRelativePath
234-
elif PathHelpers.isProtectedDeleteTarget protectedDeleteTargetNames normalizedRelativePath then
277+
elif isProtectedTarget normalizedRelativePath structuralPath then
235278
DeletePathClassification.ProtectedTarget normalizedRelativePath
236279
else
237-
let segments = normalizedRelativePath |> splitPathSegments
238-
239280
match segments with
240281
| [| singleSegment |] ->
241282
match tryParseZone singleSegment with
@@ -283,21 +324,20 @@ module ArcEntityPathRules =
283324

284325
let isGenericFileSystemTargetAllowed (relativePath: string) =
285326
let normalizedRelativePath = normalizeRelativePath relativePath
327+
let segments = normalizedRelativePath |> splitPathSegments
328+
let structuralPath = classifyStructuralArcPath segments
286329

287330
if
288331
String.IsNullOrWhiteSpace normalizedRelativePath
289332
|| PathHelpers.containsPathTraversalSegments normalizedRelativePath
290-
|| PathHelpers.isProtectedDeleteTarget protectedDeleteTargetNames normalizedRelativePath
333+
|| isProtectedTarget normalizedRelativePath structuralPath
291334
then
292335
false
293336
else
294-
let segments = normalizedRelativePath |> splitPathSegments
295-
296337
segments.Length >= 1
297338
&& (segments |> containsDisallowedGenericPathSegment |> not)
298339
&& (PathHelpers.getFileName normalizedRelativePath |> isCanonicalArcFileName |> not)
299-
&& (segments.Length <> 1 || (tryParseZone segments.[0]).IsNone)
300-
&& (segments.Length <> 2 || (tryParseZone segments.[0]).IsNone)
340+
&& (blocksGenericFileSystemTarget structuralPath |> not)
301341

302342
let tryNormalizeGenericFileSystemTarget (errorMessage: string) (relativePath: string) : Result<string, string> =
303343
let normalizedRelativePath = normalizeRelativePath relativePath
@@ -352,10 +392,11 @@ module ArcEntityPathRules =
352392
RenamePathClassification.RootTarget
353393
else
354394
let segments = normalizedRelativePath |> splitPathSegments
395+
let structuralPath = classifyStructuralArcPath segments
355396

356397
if PathHelpers.containsPathTraversalSegments normalizedRelativePath then
357398
RenamePathClassification.DisallowedTarget normalizedRelativePath
358-
elif PathHelpers.isProtectedDeleteTarget protectedDeleteTargetNames normalizedRelativePath then
399+
elif isProtectedTarget normalizedRelativePath structuralPath then
359400
RenamePathClassification.ProtectedTarget normalizedRelativePath
360401
else
361402
match segments with
@@ -403,13 +444,9 @@ module ArcEntityPathRules =
403444
false
404445
else
405446
let segments = normalizedRelativePath |> splitPathSegments
447+
let structuralPath = classifyStructuralArcPath segments
406448

407-
let isArcEntityFolder = segments.Length = 2 && (tryParseZone segments.[0]).IsSome
408-
409-
let isSafeGenericDirectoryCandidate =
410-
isGenericFileSystemTargetAllowed normalizedRelativePath
411-
412-
(isArcEntityFolder || isSafeGenericDirectoryCandidate)
449+
structuralPath <> StructuralArcPath.AddZoneRoot
413450
&& (segments |> containsDisallowedGenericPathSegment |> not)
414451
&& (PathHelpers.getFileName normalizedRelativePath |> isCanonicalArcFileName |> not)
415452

‎tests/Electron.Core/ArcFileSystemHelper.test.fs‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ open Main.Bindings.Path
77
open Main.IPC.FileSystemIO
88
open Swate.Components.Shared
99
open Swate.Electron.Shared.FileIOTypes
10-
open ARCtrl
1110
open Vitest
1211

1312
let private fsPromisesDynamic: obj = importAll "fs/promises"
@@ -158,6 +157,45 @@ Vitest.describe (
158157
})
159158
)
160159

160+
Vitest.test (
161+
"protects structural entity child folders while allowing their contents",
162+
fun () ->
163+
withAssayArc (fun arcPath -> promise {
164+
match!
165+
ArcFileSystemHelper.createFileSystemItemOnDisk
166+
arcPath
167+
(createItemRequest "assays/AssayA" "dataset" FileSystemItemKind.Folder)
168+
with
169+
| Ok createdPath -> failwith $"Expected structural folder creation to be rejected: {createdPath}"
170+
| Error error -> Vitest.expect(error.Message.Length).toBeGreaterThan (0)
171+
172+
do! createRelativeDirectoryAsync arcPath "assays/AssayA/dataset"
173+
174+
let! createdPath =
175+
createItemOrFail
176+
arcPath
177+
(createItemRequest "assays/AssayA/dataset" "raw.txt" FileSystemItemKind.File)
178+
179+
Vitest.expect(createdPath).toBe ("assays/AssayA/dataset/raw.txt")
180+
do! expectRelativePathExists arcPath createdPath true
181+
182+
match! ArcFileSystemHelper.deleteGenericFileSystemItemOnDisk arcPath "assays/AssayA/dataset" with
183+
| Ok() -> failwith "Expected structural folder deletion to be rejected."
184+
| Error error -> Vitest.expect(error.Message.Length).toBeGreaterThan (0)
185+
186+
match!
187+
ArcFileSystemHelper.renameGenericFileSystemItemOnDisk
188+
arcPath
189+
(renameRequest "assays/AssayA/dataset" "raw-data")
190+
with
191+
| Ok() -> failwith "Expected structural folder rename to be rejected."
192+
| Error error -> Vitest.expect(error.Message.Length).toBeGreaterThan (0)
193+
194+
do! deleteItemOrFail arcPath createdPath
195+
do! expectRelativePathExists arcPath createdPath false
196+
})
197+
)
198+
161199
Vitest.test (
162200
"creates generic files and folders at the ARC root",
163201
fun () ->

‎tests/Electron.Core/IpcArchitectureReview.test.fs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
module ElectronCore.IpcArchitectureReviewTests
22

3-
open Fable.Core
43
open Main.Bindings.Path
54
open Main.ArcVault
65
open Main.ArcVaultTypes
@@ -261,6 +260,8 @@ Vitest.describe (
261260
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "studies/StudyA/isa.study.xlsx").toBe (true)
262261
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "test.fsx").toBe (true)
263262
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "studies").toBe (false)
263+
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "studies/StudyA/resources").toBe (false)
264+
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "studies/StudyA/dataset/raw.tsv").toBe (true)
264265
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "README.md").toBe (false)
265266
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "../studies/StudyA/isa.study.xlsx").toBe (false)
266267
)

‎tests/Electron.Core/RenamePathRules.test.fs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,30 @@ Vitest.describe (
4545
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "assays/OldAssay").toBe (true)
4646
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "assays/OldAssay/isa.assay.xlsx").toBe (false)
4747
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "assays/OldAssay/notes/custom.txt").toBe (true)
48+
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "assays/OldAssay/dataset").toBe (false)
49+
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "assays/OldAssay/protocols").toBe (false)
50+
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "studies/StudyA/resources").toBe (false)
51+
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "assays/OldAssay/dataset/raw.txt").toBe (true)
4852
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "notes").toBe (false)
4953
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "notes/2026-06-15/foo/foo.md").toBe (true)
5054
Vitest.expect(ArcEntityPathRules.isRenamePathAllowed "test.fsx").toBe (true)
5155
)
5256

57+
Vitest.test (
58+
"native structural child matching is case-insensitive",
59+
fun () ->
60+
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "assays/OldAssay/DataSet").toBe (false)
61+
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "studies/StudyA/Protocols").toBe (false)
62+
Vitest.expect(ArcEntityPathRules.isDeletePathAllowed "studies/StudyA/Resources").toBe (false)
63+
)
64+
5365
Vitest.test (
5466
"generic filesystem targets are limited to safe non-canonical paths",
5567
fun () ->
5668
let allowedTargets = [
5769
"assays/AssayA/protocols/protocol.md"
58-
"studies/StudyA/resources"
70+
"assays/AssayA/dataset/raw.txt"
71+
"assays/AssayA/protocol"
5972
"workflows/WorkflowA/scripts/workflow.cwl"
6073
"runs/RunA/data.txt"
6174
"test.fsx"
@@ -71,6 +84,10 @@ Vitest.describe (
7184
""
7285
"assays"
7386
"assays/AssayA"
87+
"assays/AssayA/dataset"
88+
"assays/AssayA/protocols"
89+
"studies/StudyA/protocols"
90+
"studies/StudyA/resources"
7491
"assays/AssayA/isa.assay.xlsx"
7592
"assays/AssayA/isa.datamap.xlsx"
7693
"assays/AssayA/.gitattributes"
@@ -86,6 +103,16 @@ Vitest.describe (
86103
)
87104
)
88105

106+
Vitest.test (
107+
"protected entity child folders can still be used as generic filesystem parents",
108+
fun () ->
109+
Vitest.expect(ArcEntityPathRules.isGenericFileSystemParentAllowed "assays/AssayA/dataset").toBe (true)
110+
111+
Vitest
112+
.expect(ArcEntityPathRules.isGenericFileSystemParentAllowed "studies/StudyA/protocols")
113+
.toBe (true)
114+
)
115+
89116
Vitest.test (
90117
"root-level generic filesystem child paths are allowed for safe targets only",
91118
fun () ->

‎tests/Electron.Renderer/FileTreeContextMenu.test.fs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,31 @@ Vitest.describe (
257257
Vitest.expect(groupedLabels menuItems).not.toContain ("Rename")
258258
)
259259

260+
Vitest.test (
261+
"native structural entity child folders do not expose rename or delete",
262+
fun () ->
263+
let protectedPaths = [
264+
"assays/AssayA/dataset"
265+
"assays/AssayA/protocols"
266+
"studies/StudyA/protocols"
267+
"studies/StudyA/resources"
268+
]
269+
270+
protectedPaths
271+
|> List.iter (fun path ->
272+
let item = createFolderItem (PathHelpers.getNameFromPath path) (Some path)
273+
274+
let menuItemLabels =
275+
createComposedContextMenuItems (createContextMenuConfig ()) item
276+
|> groupedLabels
277+
278+
Vitest.expect(menuItemLabels).toContain ("New File")
279+
Vitest.expect(menuItemLabels).toContain ("New Folder")
280+
Vitest.expect(menuItemLabels).not.toContain ("Rename")
281+
Vitest.expect(menuItemLabels).not.toContain ("Delete")
282+
)
283+
)
284+
260285
Vitest.test (
261286
"new folder action on the ARC root requests root-level folder creation",
262287
fun () ->

0 commit comments

Comments
 (0)