UI: add filesystem browsing for allocations #7951
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There’s a lot of duplication here that’s probably worth extracting despite not meeting the three-repetitions rule because we were already simulating the API being for a task by including the task name in the URL.
This is a barely-different copypaste from the task fs acceptance tests… I’m not sure what’s the best way to accomplish this!
The assumption that we should strip the task name path prefix is no longer correct if we want to be able to browse the allocation without a task. This changes the task filesystem acceptance test files to all be nested within a task name directory, which mirrors how it works in the real setup.
Ember Asset Size actionAs of 5f82c18 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
The interface here is still subpar, with the expectations of the various things dangling off `this.` (files? directory?), but maybe some cleanup is possible…
Ember Test Audit comparison
|
There’s one left that has some duplication but it doesn’t seem worth extracting due to how much customisation it would require.
Another combination could happen if I extract something like an fs-link component 🤔
This contains some of the duplication scattered about where filesystem links are slightly different in two dimensions.
There’s no need to have these calculations scattered everywhere.
It makes more sense to concentrate these functions in the allocation adapter.
I’d LOVE for some Prettier-esque approach to this 😝
DingoEatingFuzz
approved these changes
May 29, 2020
backspace
added a commit
that referenced
this pull request
Jun 1, 2020
This partially addresses #7799. Task state filesystems are contained within a subdirectory of their parent allocation, so almost everything that existed for browsing task state filesystems was applicable to browsing allocations, just without the task name prepended to the path. I aimed to push this differential handling into as few contained places as possible. The tests also have significant overlap, so this includes an extracted behavior to run the same tests for allocations and task states.
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a filesystem browser for allocations:
Task state filesystems are contained within a subdirectory of their parent allocation, so almost everything that existed for browsing task state filesystems was applicable to browsing allocations, just without the task name prepended to the path. I aimed to push this differential handling into as few contained places as possible.
The tests also have significant overlap, so this includes an extracted
behaviorto run the same tests for allocations and task states. It’s definitely unwieldy but seemed better than having two files with almost-identical tests. Having the thorough test suite for this complex and near-identical functionality was helpful in the refactorings I completed near the end of this work.Other commentary:
fsdirectory to contain the relevant components