Code

Opened 5 years ago

Closed 13 months ago

#10214 closed New feature (duplicate)

Add 'internal' option to SEND_BROKEN_LINK_EMAILS

Reported by: mail@… Owned by: nobody
Component: Core (Other) Version: 1.0
Severity: Normal Keywords:
Cc: wrboyce Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I don't really care if people are linking to my site incorrectly, there's not much I can do about that.

I created the attached patch to make SEND_BROKEN_LINK_EMAILS accept 'internal' as a value, resulting only in an email if the referring domain is the same as the site request domain.

Attachments (2)

SBLE_internal.diff (1013 bytes) - added by mail@… 5 years ago.
SBLE_internal
SBLE_internal_81e644e.diff (8.9 KB) - added by wrboyce 2 years ago.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by mail@…

SBLE_internal

comment:1 Changed 5 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

I don't much like this setting change -- True/False/"internal" has that code smell to me -- but I wonder why the '?' in referrer bit is there to begin with. Perhaps we should just ignore all external broken links?

comment:2 Changed 5 years ago by mtredinnick

-1 on automatically ignoring external broken links. There's a lot of valuable intelligence to be gleaned from that: handling common mistakes (e.g. people not URL-encoding things correctly), spotting when URL accidentally moved or was renamed. Allowing it to be configurable is probably reasonable. Removing the facility which is actually useful today would not be so cool.

comment:3 Changed 4 years ago by ericholscher

  • Component changed from Uncategorized to Core framework

comment:4 Changed 3 years ago by SmileyChris

  • Has patch set
  • Patch needs improvement set
  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 2 years ago by aaugustin

  • Easy pickings unset
  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Design decision needed to Accepted
  • UI/UX unset

We could change SEND_BROKEN_LINK_EMAILS to accept 'none', 'internal' and 'all' as valid values.

True and False would be interpreted as 'all' and 'none'for backwards compatibility.

The change would have to be explained in the release notes.

Changed 2 years ago by wrboyce

comment:6 Changed 2 years ago by wrboyce

  • Cc wrboyce added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Updated patch to meet aaugustin's notes and include documentation/tests.

comment:7 Changed 2 years ago by aaugustin

  • Needs documentation set
  • Owner changed from nobody to aaugustin

At first sight the patch looks good. It just needs an explanation of the difference between "all" and "internal" in error-reporting.txt, and a paragraph in the release notes.

comment:8 Changed 2 years ago by aaugustin

  • Owner changed from aaugustin to nobody

comment:9 Changed 13 months ago by aaugustin

  • Resolution set to duplicate
  • Status changed from new to closed

Since my last comment, I split out this feature in its own middleware, and I deprecated SEND_BROKEN_LINK_EMAILS.

#20099 will make it easier to customize the behavior to your liking, by subclassing this middleware.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.