-
Notifications
You must be signed in to change notification settings - Fork 28
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
Problems with ${changes} in template #130
Comments
Thanks for the thorough bug report. I'll try to take a look over the weekend |
Hi @buumi. Thanks so much for your patience on this. We have a new baby in the house, and suddenly my spare time has disappeared. I've imported your template (thanks for all the details) and the changes appear correctly for a simple change. I've not had a chance to test snapshots yet, as my docker instance running tcwebhooks 1.1 has no relevant builds configured. My current thoughts are:
|
Perhaps your change list consists of more than one VCS? I don't think I've ever tested that. |
I think the problem is because value of One interesting thing I noticed in the above example was the usage of |
Hi @kichnan The code is supposed to do the serialisation for you here: Line 79 in 2675215
Yes, those commands are provided by the plugin here https://github.com/tcplugins/tcWebHooks/blob/1.1.x.x/tcwebhooks-core/src/main/java/webhook/teamcity/payload/util/WebHooksBeanUtilsVariableResolver.java There is an escape JSON function there too, but as mentioned, this should not be required. Btw, in 1.2.x, there is also support for the velocity templating engine. In velocity these functions are nicely separated, eg |
Hi @buumi I finally has a chance to look at this with a build that has changes from multiple VCS's. In my opinion we have a few issues here: "changes": [
{
"version": "a8ea5c10614406fb5a1a7d7b8321bca95fef1c9b",
"change": {
"files": [
"tcwebhooks-web-ui/src/main/resources/buildServerResources/WebHook/templateEdit.jsp"
],
"comment": "Add change count variables to template UI.\n\nAdded the following new variables so that they can be selected when\nediting a template.\n\n - \"changeFileListCount\"\n - \"maxChangeFileListSize\"\n - \"maxChangeFileListCountExceeded\"",
"username": "netwolfuk",
"vcsRoot": "https://github.com/tcplugins/tcWebHooks.git#refs/heads/master"
}
},
{
"version": "89c6500cc9f075c22b99a4b2e3a3dda187a19e3c",
"change": {
"files": [
"tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHookPayloadContent.java",
"tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChange.java",
"tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChangeBuilder.java",
"tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadJson.java",
"tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadXml.java",
"tcwebhooks-core/src/test/java/webhook/teamcity/payload/content/WebHookPayloadContentChangesTest.java"
],
"comment": "Add ability to limit or exclude vcs file list whilst building payload\n\nThis change includes the following new features:\n1. The list of changed VCS files can be disabled (0).\n2. The list of changed VCS files can be disabled if it is too large. The\ndefault value is 100 files across all changes in a build.\n3. The list can be always include (old behaviour) when the value is set\nnegative (-1).\n\nNOTE: In the cases where the change list is disabled or too large (1 & 2\nabove), the payload will contain a `null` files list.\nThis is preferable to returning an empty list, because the empty list\nimplies no actual changed files were included in the change.\nA null change list will typically be serialised to nothing in JSON or\nXML, so the json array or xml element will be missing from the payload.\nIf your endpoint is expecting these, then it should fail in this\nscenario or handle the missing field gracefully.\n\n**Enables control over the maximum number of files changed in a build\nbefore the changed file list is null.** This can be controlled in the\nthree ways below, in order of priority.\n\n1. Add a \u0027param\u0027 named \u0027maxChangeFileListSize\u0027 to a webhook config by\nediting plugin-settings.xml\n2. Add a build parameter to a buildType or project called\n\u0027webhook.maxChangeFileListSize\u0027\n3. Define a property in teamcity\u0027s internal properties file called\n\u0027webhook.maxChangeFileListSize\u0027\n\nIf the total number of files is greater than `maxChangeFileListSize`,\nthen the change list will be null. See note above.\n",
"username": "netwolfuk",
"vcsRoot": "https://github.com/tcplugins/tcWebHooks.git#refs/heads/master"
}
}
] Simply putting that into the slack payload will have two problems.
To achieve what you want, would require some post processing of he This is precisely why I have added support for the Velocity templating engine in tcWebHooks 1.2 (currently in alpha). With Velocity, we have loops, conditionals, and macros, which would make this much simpler. See #103 which even has an example of exactly what you are trying to achieve. The You might be better off using the template editor "Preview Template" button. That is less "clever" and does the JSON to HTML rendering client side, and will still display the JSON (albeit not nicely formatted if it's not valid). |
BTW, this issue can be reproduced with any change. It doesn't have to have multiple VCS's |
Expected Behavior
When adding
to template, I would expect that changes would be inserted to message.
Current Behavior
Instead I'm getting either "[]" or an error saying
An error occured building the payload preview com.google.gson.JsonSyntaxException: com.google.gson.stream.MalformedJsonException: Unterminated object at line 13 column 55 path $.attachments[0].fields[3].value
. This is from the preview window, but the actual alert also had "[]" even though the build failed and it's showing changes in TeamCity (note that those might be coming from snapshot dependencies).Steps to Reproduce (for bugs)
Add ${changes} to template and test the template.
Your Environment
Example Configuration (xml)
webhook-templates.xml:
The text was updated successfully, but these errors were encountered: