When to delete recently read entries is not configurable.Currently, it automatically deletes every entry that is older than 2 weeks...I think this should be more configurable...

Comments

nkoporec created an issue. See original summary.

nkoporec’s picture

Assigned: Unassigned » nkoporec
nkoporec’s picture

Assigned: nkoporec » Unassigned
Status: Active » Needs review
StatusFileSize
new8.2 KB

Created a patch which adds a config form where you can select the time period when the cron deletes recently read data.

icurk’s picture

Status: Needs review » Needs work

Patch works great, but I have some comments about it.

1. Configuration link is added with *links.action.yml, which is not the best way to do it. It is better to create configuration link as a tab with *.links.task.yml.
2. In RecentlyReadConfigForm change:

  protected function getEditableConfigNames() {
    return [
      'recentlyread.config',
    ];
  }

with

  protected function getEditableConfigNames() {
    return [
      'delete_config',
      'delete_time',
    ];
  }

There is no configuration object with 'recentlyread.config' name.
3. Fix coding standards in buildForm method of RecentlyReadConfigForm class (replace array() with []).

nkoporec’s picture

Status: Needs work » Needs review
StatusFileSize
new2.9 KB
new8.21 KB

Thank you for reviewing my patch.Created a new patch with fixed issues that you have found.

zterry95’s picture

one more suggestion:

add cron task to cleanup expire logs.

icurk’s picture

Status: Needs review » Reviewed & tested by the community

@zterry95 expire logs are already deleted on cron with this patch. The patch looks good.

  • boshtian committed 2517e3a on 8.x-1.x authored by nkoporec
    Issue #2953273 by nkoporec, icurk: Time to delete recently read is not...
boshtian’s picture

Status: Reviewed & tested by the community » Fixed

Great work @nkoporec and thanks @icurk for testing.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.