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

fix: boolean values are converted to 1 and 0 only on configuration page #1347

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

soleksy-splunk
Copy link
Contributor

@soleksy-splunk soleksy-splunk commented Sep 24, 2024

Issue number:
https://splunk.atlassian.net/browse/ADDON-73559

Summary

  1. Boolean values are mapped only on configuration page.
  2. Do not map boolean values when typing inside single input select (BUG)

1 justification:
Looks like splunk is using backend values mapping only on forms used on configuration page.

By backend values mapping i mean converting
values like ‘TRUE’, ‘T’, ‘Y’, ‘YES’, true will be converted into ‘1’
values like ‘FALSE’, ‘F’, ‘N’, ‘NO’, ‘NONE’, false will be converted into ‘0’

values on inputs page are saved and retrieved correctly, so I assume that on any different page than configuration that mapping is not needed.

Thats why for function used to map values getValueMapTruthyFalse there is added another parameter currentPageName. If the page is configuration values are mappend into 1 and 0 if not values are left as they are.

Changes

Please provide a summary of what's being changed

On Input page values are shared to backend correctly, only on configuration page they are mapped.

User experience

Users can use mapped till now words on inputs page.

Please describe what the user experience looks like before and after this change

Checklist

If your change doesn't seem to apply, please leave them unchecked.

@soleksy-splunk soleksy-splunk requested a review from a team as a code owner September 24, 2024 23:44
@vtsvetkov-splunk
Copy link
Collaborator

  1. can you add more justification to the PR description? More context of the problem and why it made like this
  2. Do you think all checklist items are not applicable?

@soleksy-splunk soleksy-splunk requested a review from a team as a code owner September 25, 2024 12:31
@soleksy-splunk
Copy link
Contributor Author

  1. can you add more justification to the PR description? More context of the problem and why it made like this
  2. Do you think all checklist items are not applicable?
  1. done
  2. sorry forgot to mark them, just added docs so all should be good

getValueMapTruthyFalse(currentFieldValue) === getValueMapTruthyFalse(mod.fieldValue) &&
// values are directly equal or they are equal after type convertion
(currentFieldValue === mod.fieldValue ||
// here convertion is needed as splunk keeps boolsih data on cponfiguration page as 1 and 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// here convertion is needed as splunk keeps boolsih data on cponfiguration page as 1 and 0
// here conversion is needed as splunk keeps boolish data on configuration page as 1 and 0

// here type convertion is needed as splunk keeps all data as string
// and users can put numbers or booleans inside global config
getValueMapTruthyFalse(currentFieldValue) === getValueMapTruthyFalse(mod.fieldValue) &&
// values are directly equal or they are equal after type convertion
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// values are directly equal or they are equal after type convertion
// values are directly equal or they are equal after type conversion

export function getValueMapTruthyFalse<T>(value: string | T) {
return (isFalse(value) && '0') || (isTrue(value) && '1') || value;
export function getValueMapTruthyFalse<T>(value: string | T, currentPageName?: StandardPages) {
return (
Copy link
Collaborator

Choose a reason for hiding this comment

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

can it be simplified like that?

if (currentPageName === 'configuration') {
    return (isFalse(value) && '0') || (isTrue(value) && '1') || value;
}
return value;

@vtsvetkov-splunk
Copy link
Collaborator

Looks like splunk is using backend values mapping only on forms used on configuration page.

any clue why is that? Previously we thought we cannot influence on that, but Splunk itself is not aware of our UCC pages, does it?

@soleksy-splunk
Copy link
Contributor Author

Looks like splunk is using backend values mapping only on forms used on configuration page.

any clue why is that? Previously we thought we cannot influence on that, but Splunk itself is not aware of our UCC pages, does it?

It looks like there is a different backend handling those configuration and inputs forms,

inputs is handled by

endpoint = DataInputModel(
    'example_input_one',
    model,
)

where

    @property
    def internal_endpoint(self):
        return f"data/inputs/{self.input_type}"

and configuration by

endpoint = SingleModel(
    'splunk_ta_uccexample_account',
    model,
    config_name='account'
)

where

    @property
    def internal_endpoint(self):
        return f"configs/conf-{self.conf_name}"

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.

3 participants