Bug 22193 - Mana - share comments about reports
Summary: Mana - share comments about reports
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Arnaud
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 17047
Blocks:
  Show dependency treegraph
 
Reported: 2019-01-23 15:59 UTC by Alex Arnaud
Modified: 2019-06-26 19:14 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 22193: Code refactoring/improvments (6.11 KB, patch)
2019-01-24 16:38 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 22193: Share report comments (3.60 KB, patch)
2019-01-24 16:38 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 22193: Code refactoring/improvments (6.16 KB, patch)
2019-02-01 19:46 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22193: Share report comments (3.65 KB, patch)
2019-02-01 19:46 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22193: QA follow up - return undef instead of an array if no ressource id (667 bytes, patch)
2019-06-06 09:53 UTC, Alex Arnaud
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Arnaud 2019-01-23 15:59:16 UTC
Currently, it is possible to comment subscriptions imported from Mana. We need to do the same for reports.
Patches coming soon.
Comment 1 Alex Arnaud 2019-01-24 16:38:23 UTC
Created attachment 84366 [details] [review]
Bug 22193: Code refactoring/improvments
Comment 2 Alex Arnaud 2019-01-24 16:38:29 UTC
Created attachment 84367 [details] [review]
Bug 22193: Share report comments

Test plan:
  - apply this patches,
  - download a report from Mana,
  - you should see a "Report mistake" button when you show, edit and run
    this report,
  - click on Report mistake -> New and type a comment,
  - check that it appears on the list,
  - reload the page, check the comment is still in the list,
  - click on Report mistake -> your comment,
  - ckeck the count has been incremented,
  - reload the page, the counter is still the same
Comment 3 Michal Denar 2019-01-30 20:56:12 UTC
Hi Alex,
last two lines on test plan don't work for me

More than one comments 
https://screenshots.firefox.com/bUEQMVj2BDVcwhpJ/localhost

After reload: back "New"

https://screenshots.firefox.com/f65JRGJd5rayY6ll/localhost

Mike
Comment 4 Owen Leonard 2019-02-01 19:46:42 UTC
Created attachment 84650 [details] [review]
Bug 22193: Code refactoring/improvments

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Owen Leonard 2019-02-01 19:46:44 UTC
Created attachment 84651 [details] [review]
Bug 22193: Share report comments

Test plan:
  - apply this patches,
  - download a report from Mana,
  - you should see a "Report mistake" button when you show, edit and run
    this report,
  - click on Report mistake -> New and type a comment,
  - check that it appears on the list,
  - reload the page, check the comment is still in the list,
  - click on Report mistake -> your comment,
  - ckeck the count has been incremented,
  - reload the page, the counter is still the same

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Josef Moravec 2019-02-02 09:15:02 UTC
Comment on attachment 84650 [details] [review]
Bug 22193: Code refactoring/improvments

Review of attachment 84650 [details] [review]:
-----------------------------------------------------------------

::: Koha/SharedContent.pm
@@ +141,5 @@
> +sub get_entity_comments {
> +    my ($resource, $id) = @_;
> +
> +    unless ($id) {
> +        return [];

I would like to see here an implicit undef, so just 'return;'
Comment 7 Alex Arnaud 2019-06-06 09:53:34 UTC
Created attachment 90349 [details] [review]
Bug 22193: QA follow up - return undef instead of an array if no ressource id
Comment 8 Josef Moravec 2019-06-26 19:14:39 UTC
sorry, does not apply cleanly on master