[WIP] Support special characters in reporting of Custom Attributes #11112
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
lpichler
changed the title
lpichler
force-pushed
the
support_special_characters_in_report_with_custom_attributes
branch
2 times, most recently
from
ad66628
to
32ca4da
Compare
regex changed and determination of virtual custom attributes now it is are based on prefix
in UI listing attributes in filter tab
example: method converts "MiqGroup.vms.host-disconnected" to ["MiqGroup", ["vms", "host", "disconnected"]]
Virtual custom attributes can contains special characters and thanks to that this parsing method did not work properly. This commit is fixing it.
special characters in custom attributes
lpichler
force-pushed
the
support_special_characters_in_report_with_custom_attributes
branch
from
32ca4da
to
1ce981c
Compare
|
Checked commits lpichler/manageiq@6fbce18~...1ce981c with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
lpichler
changed the title
| @@ -2,14 +2,19 @@ class MiqExpression::Field | |||
| FIELD_REGEX = / | |||
| (?<model_name>([[:upper:]][[:alnum:]]*(::)?)+) | |||
| \.?(?<associations>[a-z_\.]+)* | |||
| -(?<column>[a-z]+(_[[:alnum:]]+)*) | |||
| - | |||
There was a problem hiding this comment.
Not necessary for this PR, but if this regex is getting increasingly complex it may be better to implement a bona fide parser. User feedback is valuable - LMK if you have any opinions either way
|
@imtayadeway Can you review and let me know if you're ok with it? Thanks. |
lpichler
changed the title
|
@miq-bot add_label wip |
|
This works well with OSE labels 👍 |
yes |
|
This pull request is not mergeable. Please rebase and repush. |
This was referenced Mar 14, 2017
|
This pull request is not mergeable. Please rebase and repush. |
|
done by other referenced PRs. |
lpichler
deleted the
support_special_characters_in_report_with_custom_attributes
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fixes #10482 point 1
Purpose
Reporting with custom attributes was broken when CustomAttribute#name contains some special characters as "." or "/".
This PR is fixing in UI : clicking on filter tab in report definition and in report generation.
The common thread is basically that we have couple method which processing(and expecting) attributes (defined report) with form (for example)
Vm.hosts.disks-nameand when we have virtual custom attribute it has form
Vm.virtual_custom_attribute_manage.org/kurbenetesso it is failing in some parsing method.So I found such parsing methods and I added here some "IFs" for handling virtual custom attributes (794b986, 84a855d, 69c85ee)
I needed to change also regex in MiqExpression::Field to parse it, and I solved by prefix(6fbce18).
At this moment I am allowing to have basically "whatever" in name of custom attribute and I didn't any issue with it maybe we should limited it. ?
If we will limited it how I should tackle with custom attributes which contains forbidden characters ? (Don't display it at all ? )
cc @imtayadeway
@miq-bot assign @gtanzillo
fyi @cben @alongoldboim