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

[feature] Implement cascading parameter list #1978

Merged
merged 19 commits into from
May 14, 2024

Conversation

zuobiao-zhou
Copy link
Member

What's changed?

similar to #1976.
relate to #1705

The previous PR caused incorrect parameter display when monitoring the usage of other protocols. This PR fixes this bug.

When add a new monitor using HTTP:

2024-05-13_20-06-51.mp4

When edit a existing monitor using HTTP:

2024-05-13_20-07-15.mp4

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

@zuobiao-zhou
Copy link
Member Author

Fixed, thanks. @crossoverJie

@tomsun28 tomsun28 added the good first pull request Good for newcomers label May 13, 2024
@tomsun28 tomsun28 self-requested a review May 13, 2024 14:32
@tomsun28
Copy link
Contributor

Hi, thanks for contribution.
I found the cascading paramter rule is parent: [xxx], if the parent is new param like httpMethod, we have to update the webapp code to adapter it. This cannot adapt to the user's dynamic changes, such as adding new parent param but does not modify the front-end code.

How about this rule:

depend:
  - parentField -> value1
  - parentField2 -> value2

eg:

   - field: community
     # name-param field display i18n name
     name:
       zh-CN: SNMP community word
       en-US: SNMP Community
     # type-param field type(most mapping the html input type)
     type: text
     # when type is text, use limit to limit string length
     limit: 100
     #required-true or false
     required: false
     # param field input placeholder
     placeholder: 'Snmp community for v1 v2c'
     # dependent parameter values list
     # parent: [ 0,1 ]
     depend:
       - snmpVersion -> 0
       - snmpVersion -> 1

@zuobiao-zhou
Copy link
Member Author

Hi, thanks for contribution. I found the cascading paramter rule is parent: [xxx], if the parent is new param like httpMethod, we have to update the webapp code to adapter it. This cannot adapt to the user's dynamic changes, such as adding new parent param but does not modify the front-end code.

How about this rule:

depend:
  - parentField -> value1
  - parentField2 -> value2

eg:

   - field: community
     # name-param field display i18n name
     name:
       zh-CN: SNMP community word
       en-US: SNMP Community
     # type-param field type(most mapping the html input type)
     type: text
     # when type is text, use limit to limit string length
     limit: 100
     #required-true or false
     required: false
     # param field input placeholder
     placeholder: 'Snmp community for v1 v2c'
     # dependent parameter values list
     # parent: [ 0,1 ]
     depend:
       - snmpVersion -> 0
       - snmpVersion -> 1

Got it, I will refactor.

@zuobiao-zhou
Copy link
Member Author

Hi @tomsun28
The new rule is like:

depend:
  snmpVersion:
    - 0
    - 1

BTW, are there any other protocols that require the use of cascading parameters?

@zuobiao-zhou zuobiao-zhou changed the title [improve] Implement cascading parameter list for HTTP protocol [feature] Implement cascading parameter list May 14, 2024
@tomsun28
Copy link
Contributor

BTW, are there any other protocols that require the use of cascading parameters?

hi, great job👍, it seem no other params need cascading.

Copy link
Contributor

@tomsun28 tomsun28 left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@tomsun28 tomsun28 merged commit 4a77aaf into apache:master May 14, 2024
4 checks passed
@zuobiao-zhou zuobiao-zhou deleted the http-cascading-parameters branch May 14, 2024 06:44
@zhangshenghang zhangshenghang mentioned this pull request Jun 20, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants