When leaving out the configuration of "Unsubscribe confirmation" and "Subscribe confirmation" the module is supposed to make a fallback to the home page of the site (defined in system.site). However this results in a page not found.

The reason is that Simplenews works with Drupal 7-style paths like node/123 whereas the system.site configuration (and D8 in general) uses paths with a trailing slash like /node/123. I've attached a patch to align Simplenews with the normal D8 behaviour.

CommentFileSizeAuthor
#7 simplenews.path-consistency.3027564-interdiff-2-7.txt2.46 KBadamps
#7 simplenews.path-consistency.3027564-7.patch6.14 KBadamps
#7 simplenews.path-consistency.ONLYTESTS.3027564-7.patch831 bytesadamps
#2 simplenews-path-consistency-3027564.patch4.24 KBbjaxelsen

Comments

bjaxelsen created an issue. See original summary.

bjaxelsen’s picture

bjaxelsen commented
StatusFileSize
new simplenews-path-consistency-3027564.patch4.24 KB
omrmankar’s picture

omrmankar
Primary language English
Location Pune, Maharashtra
commented

+1

adamps’s picture

adamps commented
Status: Needs review » Reviewed & tested by the community

Patch is good thanks. The test failures look like the random unrelated ones again.

adamps’s picture

adamps commented

It would be great to have a test of the redirect. Should just be a single line added to SimplenewsSubscribeTest.php.

adamps’s picture

adamps commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests
adamps’s picture

adamps commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new simplenews.path-consistency.ONLYTESTS.3027564-7.patch831 bytes
new simplenews.path-consistency.3027564-7.patch6.14 KB
new simplenews.path-consistency.3027564-interdiff-2-7.txt2.46 KB

Here is an updated patch

  1. Added a test
  2. Did a reroll
  3. Fixed ConfirmAddForm/ConfirmRemovalForm to redirect to magic value <front> (rather than reading site config). This matches ConfirmationController, and it results in a more canonical URL e.g. rather than /user/login.

The last submitted patch, 7: simplenews.path-consistency.ONLYTESTS.3027564-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

adamps’s picture

adamps commented
Issue tags: +Plan to commit

It was the "only tests" patch that failed, so everything is looking good.

luksak’s picture

luksak
he/him
commented

#7 Works for me.

  • 34e7f98 committed on 8.x-1.x 
    Issue #3027564 by AdamPS, bjaxelsen: Redirect to homepage
adamps’s picture

adamps commented
Status: Needs review » Fixed
Issue tags: -Plan to commit

Thanks everyone

Status: Fixed » Closed (fixed)

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