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

inconsistent redirect role validator message and bug #1651

Closed
phillxnet opened this Issue Feb 13, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@phillxnet
Member

phillxnet commented Feb 13, 2017

When performing validation on redirect role changes a check is made to see if the user is attempting to changing / undo an existing btrfs redirect role. This check is successful and works as intended and as documented within the code, however the message displayed is not consistent with the validation performed.

Additionally the message displayed has no direct validation counterpart. The message in question is:

"Existing btrfs partition found; if you wish to use the redirect role either select this btrfs partition or wipe it or the whole disk and re-assign. Redirecting is only supported to a non btrfs partition when no btrfs partition exists on the same device."

The missing validation check can lead to unconstrained disk role redirection choices that will in turn lead to user confusion. I.e. it is possible to configure a redirect role on a device that contains a btrfs partition that is not redirecting to that partition. This in not an intended behaviour and leads to further buggy behaviour as the user is then presented with an import icon that will fail due to this over-site / missing validation. Re-setting the redirect role to the btrfs partition will then result in the intended and freshly constrained behaviour within the remit of the existing and working validator. And imports will then work as intended.

The code in question is:
https://github.com/rockstor/rockstor-core/blob/master/src/rockstor/storageadmin/static/storageadmin/js/views/setrole_disks.js#L115-L128

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Feb 13, 2017

Member

I have begun to look into this issue's resolution and intend to prepare a pr as soon as time constrains allow. The bug was introduced (by myself) in pr #1622 .

Member

phillxnet commented Feb 13, 2017

I have begun to look into this issue's resolution and intend to prepare a pr as soon as time constrains allow. The bug was introduced (by myself) in pr #1622 .

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Feb 14, 2017

Member

A fix and manual test procedure for this issue and it's resolution has now been developed. I will prepare and submit the pr shortly.

Member

phillxnet commented Feb 14, 2017

A fix and manual test procedure for this issue and it's resolution has now been developed. I will prepare and submit the pr shortly.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Feb 14, 2017

improve redirect role input validation #1651
Fix inconsistency between validation and user messaging.
Add additional input validation more in line with original
user messaging: ie only allow the selection of a btrfs
partition if one exists. And only in cases where no btrfs
partition is found do we allow any partition to be selected.

@schakrava schakrava closed this in #1652 Feb 19, 2017

schakrava added a commit that referenced this issue Feb 19, 2017

Merge pull request #1652 from phillxnet/1651_inconsistent_redirect_ro…
…le_validator_message_and_bug

improve redirect role input validation. Fixes #1651

@schakrava schakrava added the bug label Feb 19, 2017

@schakrava schakrava added this to the Pinnacles milestone Feb 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment