Show statistics broken down by wiki family on event page #77
Conversation
i18n/en.json
Outdated
| @@ -55,6 +55,9 @@ | |||
| "event-data": "Event data", | |||
| "event-data-for": "Event data for \"$1\"", | |||
| "event-deleted": "The event '$1' was deleted.", | |||
| "event-in-future": "Statistics can't be generated because this event takes place in future.", | |||
There was a problem hiding this comment.
"... takes place in the future" (missing 'the'?)
src/AppBundle/Model/Event.php
Outdated
| @@ -221,6 +215,18 @@ public function getCacheKey() | |||
| return (string)$this->id; | |||
| } | |||
|
|
|||
| /** | |||
| * Is the Event valid? If false, statistics will be able to be generated. | |||
There was a problem hiding this comment.
If false, statistics will not be able to be generated...
| public function isValid() | ||
| { | ||
| return $this->wikis->count() > 0 && | ||
| $this->start !== null && |
There was a problem hiding this comment.
Ther'es also 'event-in-future' which says that the statistics can't be done because the event is in the future -- do we need to add that test in the validity here?
MusikAnimal
force-pushed
the
wiki-breakdown
branch
2 times, most recently
from
5d44de0
to
5d3cef7
Compare
|
@mooeypoo Thanks for identifying all those careless errors! =p All fixed. |
MusikAnimal
force-pushed
the
wiki-breakdown
branch
from
5d3cef7
to
f529e9c
Compare
|
The commit looks good -- just one thing - the commit message (and, by proxy, the first comment in this PR) is not really right... it makes it sound all you did was remove event_valid. This commit has quite a number of changes that are tangential, like changing fonts, splitting the twig templates, adding a validation method, etc. Can you just quickly write a more summarizing commit message, for posterity? Once that's done, I think the PR can be merged! |
Some refactoring of related Twig templates. Remove unused 'event_valid' field from event table, instead doing validation in the model layer with Event::isValid(). Add message indicating event is missing required options before statistics can be generated. Change font to Roboto, and remove unused font file. Bug: https://phabricator.wikimedia.org/T192579
MusikAnimal
force-pushed
the
wiki-breakdown
branch
from
f529e9c
to
a6a85a9
Compare
|
GitHub makes the first line of the commit message the title of the PR, and the rest as the description. Not sure if you saw the whole thing? f529e9c said: but I just forced pushed back, adding even more info :) Now it's at a6a85a9 |
|
... I think I was looking at an earlier one? In any case, we can also edit the top comment on the PR so that when/if we look at it on Github it's all there. |
Some refactoring of related Twig templates.
Remove unused 'event_valid' field from event table, instead doing
validation in the model layer with Event::isValid().
Add message indicating event is missing required options before
statistics can be generated.
Change font to Roboto, and remove unused font file.
Bug: https://phabricator.wikimedia.org/T192579