add_daily_stats #617
Closed
add_daily_stats #617
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bodom91
force-pushed
the
Admin_daily_report
branch
from
dbcab17 to
95b68b0
Compare
php-coder
requested changes
Aug 1, 2017
| <a th:href="@{${SITE_EVENTS_PAGE}}" th:text="#{t_watch_suspicious_activities}" href="events.html">view suspicious activities</a> | ||
| </li> | ||
| <li sec:authorize="hasAuthority('VIEW_DAILY_STATS')"> | ||
| <a th:href="@{${DAILY_STATISTICS}}" th:text="#{t_daily_statistics}" href="../report/daily">view daily statistics</a> |
There was a problem hiding this comment.
Because we don't have report/daily page then let's use href="javascript:void(0)" here.
| ); | ||
| report.setMissingCsrfCounter(missingCsrfCounter); | ||
|
|
There was a problem hiding this comment.
Could you remove these whitespace-only changes?
| private final MailService mailService; | ||
| private final CronService cronService; | ||
|
|
||
| @RequestMapping(Url.DAILY_STATISTICS) |
| public ReportController getReportController() { | ||
| return new ReportController( | ||
| servicesConfig.getMailService(), | ||
| servicesConfig.getCronService() |
There was a problem hiding this comment.
Reduce indentation for this line, please.
| public static final String INDEX_PAGE = "/"; | ||
| public static final String ROBOTS_TXT = "/robots.txt"; | ||
| public static final String SITEMAP_XML = "/sitemap.xml"; | ||
|
|
There was a problem hiding this comment.
Avoid white-space only changes where it's possible.
| ); | ||
| printWriter.println(stats); | ||
| } catch (IOException e) { | ||
| LOG.error("Can't get Daily Reports", e.getMessage()); |
| private final CronService cronService; | ||
|
|
||
| @RequestMapping(Url.DAILY_STATISTICS) | ||
| public void adminDailyReport(HttpServletResponse response) { |
There was a problem hiding this comment.
s/adminDailyReport/showDailyReport/
| response.setCharacterEncoding("UTF-8"); | ||
|
|
||
| try { | ||
| PrintWriter printWriter = response.getWriter(); |
| cronService.getDailyStatistics() | ||
| ); | ||
| printWriter.println(stats); | ||
| } catch (IOException e) { |
| private final MailService mailService; | ||
| private final CronService cronService; | ||
|
|
||
| @RequestMapping(Url.DAILY_STATISTICS) |
There was a problem hiding this comment.
Could you test for me the following (inspired by https://stackoverflow.com/a/7672903/352708):
- add
consumesparameter to@GetMapping(see https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/web/bind/annotation/RequestMapping.html#produces--) - add
@ResponseBodyannotation - return
Stringfrom the controller
Does it work the same as the current approach?
…d: port to Robot Framework. Addressed to php-coder#530 No functional changes.
…d: port to Robot Framework. Addressed to php-coder#530 No functional changes.
I don't want to port it to Robot Framework because a) it will be hard b) now I don't think that this check was so useful
Addressed to php-coder#530 No functional changes.
Should be in 7cfaf2d commit. No functional changes.
Should be in 29c8230 commit. No functional changes.
Addressed to php-coder#530 No functional changes.
Addressed to php-coder#530 No functional changes.
Addressed to php-coder#530 No functional changes.
…ory with name that differs only by case. Fix php-coder#446
…ry with name that differs only by case. Fix php-coder#447
No code changes.
No functional changes.
No functional changes.
Addressed to php-coder#530 No functional changes.
Addressed to php-coder#530 No functional changes.
Addressed to php-coder#530 No functional changes.
Addressed to php-coder#530 No functional changes.
Addressed to php-coder#530 No functional changes.
…igation is used. No functional changes.
Addressed to php-coder#530 No functional changes.
…ework. Addressed to php-coder#530 No functional changes.
Changelog: https://projectlombok.org/changelog.html No functional changes.
Changelogs: - http://forum.thymeleaf.org/Thymeleaf-3-0-3-and-Spring-5-integration-JUST-PUBLISHED-td4030168.html - https://github.com/thymeleaf/thymeleaf/blob/thymeleaf-3.0.3.RELEASE/ChangeLog.txt - https://github.com/thymeleaf/thymeleaf-spring/blob/thymeleaf-spring4-3.0.3.RELEASE/thymeleaf-spring4/ChangeLog.txt No functional changes.
Prior this change it was failing with the following error:
/home/travis/.travis/job_stages: line 567: expected `)'
/home/travis/.travis/job_stages: line 567: syntax error near `-a'
/home/travis/.travis/job_stages: line 567: ` if [[ ($TRAVIS_BRANCH = prod) && ("$SPRING_PROFILES_ACTIVE" = "travis" -a "$TRAVIS_PULL_REQUEST" = "false") ]]; then'
Correction for d5289a4 commit.
I don't want to port it to the Robot Framework because I'm trying to make tests to test a behavior instead of the pages state. No functional changes.
No functional changes.
This reverts commit 08bb9a4. Reverting this change won't harm but it's possible that it solves "Cannot find a merge base between danger_base and danger_head" error from Danger.
This reverts commit ad3a23e.
…e between danger_base and danger_head" error." This reverts commit f0db775.
…t modified. Now the checks will be run only when the files with the following patterns were modified: - sortpom: pom.xml - enforcer: pom.xml - checkstyle: pom.xml, *.java, *.properties, checkstyle.xml, checkstyle-suppressions.xml - findbugs: pom.xml, *.java, findbugs-filter.xml - pmd: pom.xml, *.java, pmd.xml - unit tests: pom.xml, *.java, *.groovy - license: pom.xml, *.java, *.groovy, license_header.txt - codenarc: pom.xml, *.groovy - jasmine: pom.xml, *.js - rflint: .travis.yml, *.robot - bootlint: .travis.yml *.html - html5validator: .travis.yml *.html Integration tests are executed always. Fix php-coder#423
No functional changes.
Changelog: https://projectlombok.org/changelog No functional changes.
Changelog: https://projectlombok.org/changelog No functional changes.
No functional changes.
…upport.beanvalidation Addressed to php-coder#591 No functional changes.
Should be in ba82718 commit. No code changes.
Should be in ba82718 commit. No code changes.
This also updates Spring Framework to 4.3.9.RELEASE Changelog: - https://spring.io/blog/2017/06/08/spring-boot-1-4-7-available-now - https://spring.io/blog/2017/06/07/spring-framework-4-3-9-available-now No functional changes.
|
@bodom91 Looks like you tried to use rebase but did it wrong. Typically it should be about 3 steps to refresh a working branch:
|
php-coder
requested changes
Aug 2, 2017
| * @author Maxim Shestakov | ||
| */ | ||
| @Controller | ||
| @RestController |
| } catch (IOException ex) { | ||
| LOG.error("Can't get daily report", ex.getMessage()); | ||
| } | ||
| @GetMapping(value = Url.DAILY_STATISTICS, produces = {"text/plain; charset=UTF-8"}) |
There was a problem hiding this comment.
-@GetMapping(value = Url.DAILY_STATISTICS, produces = {"text/plain; charset=UTF-8"})
+@GetMapping(path = Url.DAILY_STATISTICS, produces = "text/plain; charset=UTF-8")| } | ||
|
|
||
| private String getTextOfDailyStatisticsMail(AdminDailyReport report) { | ||
| public String getTextOfDailyStatisticsMail(AdminDailyReport report) { |
There was a problem hiding this comment.
Please, add @Override annotation to this method.
There was a problem hiding this comment.
Also this method should be placed before private methods.
php-coder
reviewed
Aug 2, 2017
| LOG.error("Can't get daily report", ex.getMessage()); | ||
| } | ||
| @GetMapping(value = Url.DAILY_STATISTICS, produces = {"text/plain; charset=UTF-8"}) | ||
| @ResponseBody |
There was a problem hiding this comment.
Just for your information: @RestController is essentially the same as @Controller and @ResponseBody. Having both is superfluous.
|
I did it |
Generated by 🚫 Danger |
|
But why? You can just push with |
I see that your master is OK. And I can't see what was in the |
|
Superseded by #618 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sorry for my delay.