remove strong type on SendFeedbackForFeature function #18547
Conversation
remove strong type on functions
peterhashair
changed the title
peterhashair
added
Needs Review
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
github-actions
bot
added
the
Stale
update int to string
| @@ -31,11 +31,11 @@ class API extends \Piwik\Plugin\API | |||
| * config: "feedback_email_address". | |||
| * | |||
| * @param string|null $featureName The name of a feature you want to give feedback to. | |||
| * @param int $like Whether you like the feature or not | |||
| * @param string|null $like Whether you like the feature or not | |||
There was a problem hiding this comment.
I guess this is a bit strange to use string|null and then set the default to $like = false.
plugins/Feedback/API.php
Outdated
| * @param string|null $choice Multiple choice option chosen | ||
| * @param string|null $message A message containing the actual feedback | ||
| */ | ||
| public function sendFeedbackForFeature(?string $featureName, int $like, ?string $choice, ?string $message = null) | ||
| public function sendFeedbackForFeature($featureName, $like = false, $choice = null, $message = null) |
There was a problem hiding this comment.
Maybe this should be $like = null
| @@ -257,7 +257,7 @@ export default defineComponent({ | |||
| AjaxHelper.fetch({ | |||
| method: 'Feedback.sendFeedbackForFeature', | |||
| featureName: this.title, | |||
| like: this.like ? '1' : '0', | |||
There was a problem hiding this comment.
Suggest keeping this line as is otherwise both 'true' and 'false' may be true in php
update type
|
@justinvelluppillai @peterhashair didn't you see that the vue build action failed for this PR due to some code formatting issues? Would be good to always check that before merging, as now the action fails for all PRs after merging in latest changes... |
|
@sgiehl sorry missed that - yes certainly the build should always be required to pass at least before merging. |
|
I have pushed a fix here: #18525 |
|
@sgiehl yes, sorry, I forgot to update the VUE format. |
Description:
Fixes: #18537
remove strong type on SendFeedbackForFeature function
Review