Skip to content
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

Let user specify how many rows will be displayed in Scheduled Reports. #6607

Conversation

czolnowski
Copy link
Contributor

I have simply change const into config value and describe them in global.ini.php

@tsteur
Copy link
Member

tsteur commented Nov 6, 2014

We really need to find a way to let plugins configure things without having to edit global.ini.php which should ideally not be used for this. It adds a dependency from core to a plugin but all the logic should be contained in the plugin otherwise we end up having many problems. Maybe in this case the Settings API could be used?

Of course having everything in one config file is nice for the users. Maybe plugins could also provide their own "global.ini.php" which could be merged somehow with our "global.ini.php". "somehow" :) Or something similar...
Edit: I am going to create an issue for this...

@czolnowski
Copy link
Contributor Author

Settings API sounds very good for me! I'll try to write some code to solve this and attach to this PR, so please, hold with merging. :)

; scheduled reports truncate limit
; the report will be rendered with the first 23 rows and will aggregate other rows in a summary row
; 23 rows table fits in one portrait page
schedule_reports_truncate = 23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be scheduled

@mattab
Copy link
Member

mattab commented Nov 7, 2014

@czolnowski thomas created issue at #6609 but you don't need to solve it now, let's stay focused on this small feature for this PR

@mattab mattab added this to the Piwik 2.9.0 milestone Nov 7, 2014
@mattab mattab added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Nov 7, 2014
@tsteur
Copy link
Member

tsteur commented Nov 7, 2014

Hacks / adding dependencies shouldn't be justified just because it is a small feature. Then we'll always have this mess ;) And using Settings API should be easy to use

@mattab
Copy link
Member

mattab commented Nov 7, 2014

@tsteur of course I agree that we should find better solution directly, but here we have already done this before (see setting scheduled_reports_replyto_is_user_email_and_alias) so IMO it's acceptable for Marcin to make this PR and us to merge it. also of course you're welcome to put #6609 in Short term

@mattab mattab changed the title As Piwik user I would like to control how many rows will consist report in Scheduled Reports. Let user specify how many rows will be displayed in Scheduled Reports. Nov 7, 2014
@czolnowski
Copy link
Contributor Author

@mattab: I have renamed this config parameter according to your guidelines.

@mattab
Copy link
Member

mattab commented Nov 8, 2014

Cheers! fyi it's not 'my' guideline but it's to be consistent with existing setting just above in the file

mattab pushed a commit that referenced this pull request Nov 8, 2014
…ncate-limit-into-config

Let user specify how many rows will be displayed in Scheduled Reports.
@mattab mattab merged commit 3e81595 into matomo-org:master Nov 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants