[2.x] Move email_registration settings in its own fieldset

Created on 10 October 2023, 9 months ago
Updated 23 October 2023, 8 months ago

Problem/Motivation

Currently, the settings are all inside the "Registration and cancellation" fieldset. This only makes partially sense and because we do not want to split these settings, it would be best, to simply put all module settings in its own "Email Registration" fieldset.

Steps to reproduce

Proposed resolution

Put all module settings in its own "Email Registration" fieldset.

Remaining tasks

User interface changes

API changes

Data model changes

📌 Task
Status

Fixed

Version

2.0

Component

Code

Created by

🇩🇪Germany Grevil

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Grevil
  • Status changed to Needs work 9 months ago
  • 🇩🇪Germany LRWebks Porta Westfalica

    Starting with this now!

  • @lrwebks opened merge request.
  • Assigned to LRWebks
  • 🇩🇪Germany LRWebks Porta Westfalica
  • Status changed to Needs review 8 months ago
  • 🇩🇪Germany LRWebks Porta Westfalica

    Here we go, works for me! Please review!

  • 🇩🇪Germany Anybody Porta Westfalica

    @LRWebks: Nice! Here comes the trap: Changing form structure may lead to broken validation / submit logic (that's based on the #tree property on form arrays). If it is a tree, you may need to also adjust the handling logic. Same may happen for configs!

    So please try, if it still works as expected. Don't rely on test coverage for modules and please provide information if and how much you tested, when setting an issue to "Needs review", please. So we're informed how safe it is and if it needs further (manual or automated) testing. Welcome to the next level :P

  • 🇩🇪Germany Anybody Porta Westfalica
  • Status changed to Needs work 8 months ago
  • 🇩🇪Germany Anybody Porta Westfalica
  • Status changed to Needs review 8 months ago
  • 🇩🇪Germany LRWebks Porta Westfalica

    As far as I have searched through the code, all classes that use these settings get them directly from the configFactory via $container->get('config.factory')->get('email_registration.settings'), the previous fieldset that they were in was never mentioned anywhere, so I guess the fieldset doesn't matter if it is done this way?
    I see no need to change anything - manual tests work, automated tests work...

  • Assigned to Grevil
  • 🇩🇪Germany Anybody Porta Westfalica

    @Grevil: Please review then :)

  • Issue was unassigned.
  • Status changed to RTBC 8 months ago
  • 🇩🇪Germany Grevil

    LGTM! Sry, I said "fieldset", but I meant "details" wrapper. Small adjustments, everything else is perfect!

  • Status changed to Fixed 8 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024