novuhq / novu

Open-Source Notification Platform. Embeddable Notification Center, E-mail, Push and Slack Integrations.
https://novu.co
Other
34.47k stars 3.51k forks source link

[NV-1598] 🐛 Bug Report: Template variables are given a default empty string value even when nothing is entered in the default value form #2636

Open spakanati opened 1 year ago

spakanati commented 1 year ago

📜 Description

When creating a template, even when you do not set a default variable, the value is set to an empty string.

👟 Reproduction steps

Create a template and use a variable named testVar but do not enter a default value. Send testVar in your payload as null or undefined.

👍 Expected behavior

Novu should not overwrite with an empty string if there is no default value set.

👎 Actual Behavior with Screenshots

Note that when inspecting the "payload" in the activity feed Novu will have sent an empty string as a default value for testVar.

You can also check by getting the template from the API and seeing this in the variables:


            {
              "name": "testVar",
              "type": "String",
              "required": false,
              "defaultValue": "",
              "_id": "63d5ccb3a9b6cc61d4cdce39"
            },```

### 📃 Provide any additional context for the Bug.

_No response_

### 👀 Have you spent some time to check if this bug has been raised before?

- [X] I checked and didn't find similar issue

### 🏢 Have you read the Contributing Guidelines?

- [X] I have read the [Contributing Guidelines](https://github.com/novuhq/novu/blob/main/CONTRIBUTING.md)

### Are you willing to submit PR?

None

<sub>[NV-1598](https://linear.app/novu/issue/NV-1598/🐛-bug-report-template-variables-are-given-a-default-empty-string)</sub>
scopsy commented 1 year ago

So you mean using the JS truthy value assertion for empty string, false and null? I think we can have both, before saving clean it, and also when actually making the defaults in runtime remove "falsy" values. What do you think?