Haven't tested it but LGTM. My usual comment: Would be nice to extract some private methods into other classes making them public and better (independent) testable but up to you :)

Would be nice to extract some private methods into other classes making them public and better (independent) testable but up to you

Perhaps you could be more specific? If you have to repeat yourself, it means your method of communication isn't having the desired effect.

One method is public in the command and is tested in a unit test. The others are only for processing input options. I don't know where to put those where they wouldn't just create clutter. Is there some symfony command way of defining those (ie, something like the DialogHelper) I am not aware of?

I didn't mean the input ones sorry should have mentioned it in the code. Will leave some comments. I'm just personally trying to have as many public methods as possible so the methods are testable and reusable - if needed. Having one or two more classes should rather remove clutter from one big class instead of adding new clutter.

@tsteur Ok, I understand what you mean now. Some of your suggestions I can look into with this PR, others are unrelated. Also I don't consider them to be high value work, so I will probably create issues for them in the 3.0 branch

I think most of these suggestions have to do w/ handling console input, really. Eg, getting & validating date ranges + idsites, etc. Perhaps we can create a custom symfony console helper for reuse.

@tsteur Created an issue here: #8844

I think not all are console related. Eg the idSites check can be used in various scenarios but a console helper will be certainly nice 👍

I think not all are console related.

Mentioned this at the end of the issue I created (when I realized this ;)). I'm not sure what a more "universal" concept would be though. Anyway not something I will work on now :)

@tsteur Moved some code into other places and added more tests, can you check that you see nothing wrong w/ the last couple commits?

a new command core:invalidate-report-data that provides a user friendly way to invalidate archive data (as an alternative to using the API multiple times).

this command is useful, and it would be awesome to push the functionality down to the API. Can we enhance existing API call (documented in this FAQ and commonly used), or do we need to create new API? then it would make sense to even validate the inputs in API and move logic (eg. friendly error messages) from command to API. what do you think?

Looking at the CoreAdminHome.invalidateArchivedReports core code (not this PR) there is a bug: when invalidating a day, it currently doesnt invalidate week/month/year but should (since one of the day has changed).

As per discussion:

• we need "upwards cascading" as default behavior (for data correctness)
• we need downwards cascading, on-demand (eg. via new invalidateArchivedReports API parameter cascade=1)
  • we can move more command logic to the API itself (bonus: many use this API already, and CLI access is in general is more difficult)

@mattab what about invalidating single segment?

@quba sounds good to me...eg. &segment=pageUrlxxyzz would invalidate only archives for this segment. Would be especially easy to use when we do #8209 then we could simply use &segment=idSegment==6 or so

@diosmosis do you maybe want to look at adding segment param along with other changes?

@diosmosis do you maybe want to look at adding segment param along with other changes?

Covered in #8858

related: New plugin: provide simple UI to invalidate old reports #8942

Should we maybe put the command core:invalidate-report-data + tests already in that plugin so we don't have to break anything later? Or is the command too useful to have it in a plugin on the Marketplace?

Left a few comments but looks good to me

Should we maybe put the command core:invalidate-report-data + tests already in that plugin so we don't have to break anything later? Or is the command too useful to have it in a plugin on the Marketplace?

@mattab can you answer this?

Should we maybe put the command core:invalidate-report-data + tests already in that plugin so we don't have to break anything later? Or is the command too useful to have it in a plugin on the Marketplace?

Command is too useful to have in a plugin, it must be in core for now.

Updated FAQ, added:

• Alternatively you may use the core:invalidate-report-data console commmand:

  ./console core:invalidate-report-data --dates=2012-01-01,2011-10-15 --sites=1,3,5 --dry-run

• when invalidating report for a period eg. week it automatically invalidates all periods that include this one, eg. the enclosing month and year. We say the invalidation cascades up. By default, periods that are included are not invalidated (invalidating month will not re-process the days and weeks within the month). You may also force Piwik to invalidate inner periods by setting the parameter &cascadeDown=1.