-
Notifications
You must be signed in to change notification settings - Fork 855
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
add keyFormat and format query params to the view message link for topics #278
add keyFormat and format query params to the view message link for topics #278
Conversation
c2233b6
to
d3e1306
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Do you think it is possible to write a simple unit test to verify this bug?
Not sure what / how to test this. It's controller logic and I don't see any other tests for them. Also not familiar with the test framework yet so I'll have to have a look later. |
@jorkzijlstra ok for now I think it should be fine. Can you just confirm that this change doesn't have any effect if |
@davideicardi I can confirm that since that is also what we are actually running. It will just revert to the |
@@ -124,7 +124,7 @@ | |||
<tbody> | |||
<#list topic.partitions as p> | |||
<tr> | |||
<td><a href="<@spring.url '/topic/${topic.name}/messages?partition=${p.id}&offset=${p.firstOffset}&count=100'/>">${p.id}</a></td> | |||
<td><a href="<@spring.url '/topic/${topic.name}/messages?partition=${p.id}&offset=${p.firstOffset}&count=100&keyFormat${keyFormat}=&format=${format}'/>">${p.id}</a></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that sould be keyFormat=${keyFormat}&format=${format}
I noticed that the view message for topic did not have the keyFormat and format query params specified which causes the message to be de-serialized in the "DEFAULT" format instead of the message.format specified in the config.
Fix #239