Page MenuHomePhorge

Fix greylisting setting
ClosedPublic

Authored by machniak on Aug 25 2021, 12:28 PM.
Tags
None
Referenced Files
F12986169: D2815.id8056.diff
Tue, Jun 25, 2:47 PM
F12986148: D2815.id8137.diff
Tue, Jun 25, 2:45 PM
F12986115: D2815.id8086.diff
Tue, Jun 25, 2:39 PM
F12986097: D2815.id8062.diff
Tue, Jun 25, 2:36 PM
F12986091: D2815.id.diff
Tue, Jun 25, 2:35 PM
F12986022: D2815.id8068.diff
Tue, Jun 25, 2:24 PM
F12985989: D2815.id8080.diff
Tue, Jun 25, 2:21 PM
F12982548: D2815.diff
Tue, Jun 25, 3:30 AM
Subscribers
Restricted Project

Details

Reviewers
mollekopf
Group Reviewers
Restricted Project
Commits
rKa53ecb760738: Fix greylisting setting
Test Plan

./phpunit

Diff Detail

Repository
rK kolab
Branch
dev/greylist-tests
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 36409
Build 14425: arc lint + arc unit

Event Timeline

machniak created this revision.
  • Small perf. improvement, skip redundant query
  • Performance optimizations
  • Move greylisting setting back to user_settings, cleanup
  • Remove redundant ->limit(1)
mollekopf added inline comments.
src/app/Policy/Greylist/Request.php
113

I would say if there is no recipient set $enabled = false so we bail out after capturing the sender statistics?

@vanmeeuwen Should we capture sender statistics if the recipient is not found,? should we greylist if the recipient is not found?

src/database/migrations/2021_08_25_120000_drop_greylist_settings_table.php
28

I suppose in principle we have to (we'd be running code expecting the table).

Looks good to me. We should probably also create a policycontrollertest, to test if this actually works, but that doesn't need to be in this diff.

Looks good to me. We should probably also create a policycontrollertest, to test if this actually works, but that doesn't need to be in this diff.

There are some controller tests in tests/Feature/Stories, but I agree not enough of the controller alone.

This revision is now accepted and ready to land.Aug 27 2021, 9:20 AM
This revision was automatically updated to reflect the committed changes.