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

Swap order of newVolumes representation in summary table #2933 #2942

Conversation

FroggyFlox
Copy link
Member

Fixes #2933
@phillxnet, @Hooverdan96, ready for review

The internal:external representation of volumes:shares in the summary table showed at the end of the Add Storage to a Rock-On process was the inverse to what it should have been (inverse of table headers). The commit in this Pull Request (PR) swaps the order in the underlying Handlebar helper used to display this in the summary table to correct this.

See linked issue for more details on the cause of the problem corrected here as well as on the testing of this PR. Briefly:

Before this PR

The internal:external representation was swapped in the summary table displayed at the end of the "Add Storage" to an already-installed Rock-On; see below the emby-videofiles Share:

PR2933_PRE_add_storage_summary

After this PR

The same process leads to the correct display of the internal:external representations:
PR2933_POST_add_storage_summary

Clicking "Next" here and following through the procedure leads to the Rock-On being updated successfully as displayed by the new Rock-On info view:
image

Inspecting the underlying docker container confirms this as well:

buildvm:/opt/rockstor # docker inspect embyserver
(...)
        "Mounts": [
            {
                "Type": "bind",
                "Source": "/mnt2/emby-media",
                "Destination": "/media",
                "Mode": "",
                "RW": true,
                "Propagation": "rprivate"
            },
            {
                "Type": "bind",
                "Source": "/mnt2/emby-videofiles",
                "Destination": "/videofiles",
                "Mode": "",
                "RW": true,
                "Propagation": "rprivate"
            },
            {
                "Type": "bind",
                "Source": "/etc/localtime",
                "Destination": "/etc/localtime",
                "Mode": "ro",
                "RW": false,
                "Propagation": "rprivate"
            },
            {
                "Type": "bind",
                "Source": "/mnt2/emby-conf",
                "Destination": "/config",
                "Mode": "",
                "RW": true,
                "Propagation": "rprivate"
            }
        ],
(...)

All tests still pass:

buildvm:/opt/rockstor # cd /opt/rockstor/src/rockstor/ && poetry run django-admin test ; cd /opt/rockstor
Found 295 test(s).
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
.......................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 295 tests in 17.953s

OK
Destroying test database for alias 'default'...
Destroying test database for alias 'smart_manager'...

The internal:external representation of volumes:shares in the summary
table showed at the end of the Add Storage to a Rock-On process was
the inverse to what it should have been (inverse of table headers).

This commit adjusts the Handlebars helper used to display the newly
added volume to swap the internal:external representations.
@FroggyFlox FroggyFlox marked this pull request as ready for review December 28, 2024 13:58
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@FroggyFlox Thanks for seeing to this remaining reversal in our Rock-ons summary/settings. As always much apprecaited

From an rpm build using this branch I used the Syncthing Rock-on and received the following summary table after adding a volume:

New-Volume-summary

And confirmed our settings, post this config, maintained their prior correctness:

New-Volume-applied-settings

The mappings were also evident via:

docker inspect syncthing
...
        "HostConfig": {
            "Binds": [
                "/mnt2/syncthing-config:/config",
                "/mnt2/syncthing-data:/home/syncthing/Sync",
                "/mnt2/syncthing-added-volume:/added-volume",
                "/etc/localtime:/etc/localtime:ro"
            ],

And the /added-volume mapping to its Share (given correct UID) could be successfully added as a "Folder" via the Syncthing Web-UI:

Web-UI_added-volume

My apologies for missing this remaining inadvertent reversal in my last visit to this area of the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants