Skip to content
Snippets Groups Projects

Issue #3318112: Move "Block layout" from Structure to Appearance

Open Benji Fisher requested to merge issue/drupal-3318112:3318112-appearance-block into 10.1.x

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
157 * Provides a redirect and optionally adds warning messages.
158 *
159 * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
160 * The current route match. The route name should end in '.bc'.
161 * @param \Symfony\Component\HttpFoundation\Request $request
162 * The current request.
163 * @param string $change_record
164 * The URL of the change record, to be included in the log message.
165 * @param \Psr\Log\LoggerInterface|null $logger
166 * (optional) A logger for the warning message.
167 * @param \Drupal\Core\Messenger\MessengerInterface|null $messenger
168 * (optional) A messenger for the warning message.
169 *
170 * @return \Symfony\Component\HttpFoundation\RedirectResponse
171 */
172 protected function redirectWithWarning(RouteMatchInterface $route_match, Request $request, string $change_record, ?LoggerInterface $logger = NULL, ?MessengerInterface $messenger = NULL): RedirectResponse {
  • 129 $this->assertSession()
    130 ->pageTextNotContains($old_path);
    131 }
    132 }
    133
    134 /**
    135 * Provides data for testBlockPageRedirects.
    136 */
    137 public function providerTestBlockPageRedirects(): array {
    138 return [
    139 'block.admin_demo' => [
    140 '/admin/structure/block/demo/{theme}',
    141 '/admin/appearance/block/demo/{theme}',
    142 FALSE,
    143 ],
    144 'entity.block.delete_form' => [
    • Comment on lines +138 to +144

      Our redirect code has handling for query strings, but none of our tests cover that, should we expand this to add at least one test case with a query string?

    • Please register or sign in to reply
  • Looking great, one comment about making the redirect more generic - now we have 3 use-cases we're hitting the rule of 3. And one comment about adding an extra test case for some untested functionality in the redirect.

  • Benji Fisher added 52 commits

    added 52 commits

    • f652fc57...92f58710 - 34 commits from branch project:10.1.x
    • 92f58710...0ca6a237 - 8 earlier commits
    • b8d70dd2 - Add a test for the BC redirects
    • aabaa0b0 - Use a single controller for all 7 routes
    • dbd6ca68 - Get route name from route_match and refactor
    • b8faba95 - Link to the change record, not the issue, in the deprecation message
    • d333aad2 - Make the helper function protected
    • 1d5abb40 - Update test to match consolidated redirect function
    • 7407f968 - Another issue
    • ef398c8c - Test the status message, not the whole page text
    • bf994975 - Test the query string
    • 8767e6c0 - Give the testbot a break

    Compare with previous version

  • Benji Fisher added 1 commit

    added 1 commit

    • 048af582 - Revert "Give the testbot a break"

    Compare with previous version

  • Benji Fisher added 77 commits

    added 77 commits

    • 048af582...2420d38b - 58 commits from branch project:10.1.x
    • 2420d38b...1a67bf8f - 9 earlier commits
    • d8e66441 - Use a single controller for all 7 routes
    • c23f5ef6 - Get route name from route_match and refactor
    • e519b945 - Link to the change record, not the issue, in the deprecation message
    • bb4cf2d3 - Make the helper function protected
    • 9ac17fd0 - Update test to match consolidated redirect function
    • 092adf1a - Another issue
    • 1da1a4e1 - Test the status message, not the whole page text
    • e90c3569 - Test the query string
    • 40e180f6 - Give the testbot a break
    • c0f86d17 - Revert "Give the testbot a break"

    Compare with previous version

  • Benji Fisher added 21 commits

    added 21 commits

    • c0f86d17...d1faffc7 - 3 commits from branch project:10.1.x
    • d1faffc7...e9639b3a - 8 earlier commits
    • 0f44840d - Add a test for the BC redirects
    • 734f2918 - Use a single controller for all 7 routes
    • 182fbb92 - Get route name from route_match and refactor
    • ef0d5529 - Link to the change record, not the issue, in the deprecation message
    • 4329e1bd - Make the helper function protected
    • 05aff6c7 - Update test to match consolidated redirect function
    • ffa57b61 - Test the status message, not the whole page text
    • 91ec1e8e - Test the query string
    • 4798cda2 - Give the testbot a break
    • 1bacaf49 - Revert "Give the testbot a break"

    Compare with previous version

  • Benji Fisher added 1 commit

    added 1 commit

    • cec6744d - Update paths in a fwe more tests (FJS and Nightwatch)

    Compare with previous version

  • Benji Fisher added 52 commits

    added 52 commits

    • cec6744d...0c59ee88 - 32 commits from branch project:10.1.x
    • 0c59ee88...eb3164a4 - 10 earlier commits
    • fb96ed51 - Get route name from route_match and refactor
    • 8457acba - Link to the change record, not the issue, in the deprecation message
    • b67e8807 - Make the helper function protected
    • 2da86c7c - Update test to match consolidated redirect function
    • 4a43f756 - Test the status message, not the whole page text
    • adf0b17e - Test the query string
    • c21a449e - Give the testbot a break
    • 9a5469e7 - Revert "Give the testbot a break"
    • b0563f7e - Update paths in a fwe more tests (FJS and Nightwatch)
    • e6f6ed97 - Add a temporary link under Structure in the admin menu

    Compare with previous version

  • 1 1 block.admin_display:
    2 2 title: 'Block layout'
    3 parent: system.admin_structure
    3 parent: system.themes_page
    4 4 description: 'Configure what block content appears in your site''s sidebars and other regions.'
    5 5 route_name: block.admin_display
    6 block.admin_display.bc:
    7 title: 'Block layout'
    8 parent: system.admin_structure
    9 description: 'Use the link under Appearance.'
    • Maybe something like:

      "Block layout has moved to Appearance, this link will be removed in a future update."

      Trying to phrase it in a way that communicates where the link has been moved to and that this link will go away in the future, without communicating Drupal-specific information, as we typically don't display Drupal version numbers in the admin UI to users who may not understand what that means.

    • Benji Fisher changed this line in version 16 of the diff

      changed this line in version 16 of the diff

    • Please register or sign in to reply
  • Benji Fisher added 25 commits

    added 25 commits

    • e6f6ed97...53f13043 - 4 commits from branch project:10.1.x
    • 53f13043...6820fad3 - 11 earlier commits
    • d8d28641 - Link to the change record, not the issue, in the deprecation message
    • 4251e0a3 - Make the helper function protected
    • f508b49b - Update test to match consolidated redirect function
    • bfecbd60 - Test the status message, not the whole page text
    • 97529460 - Test the query string
    • 980c7541 - Give the testbot a break
    • 4144a0c7 - Revert "Give the testbot a break"
    • faec9462 - Update paths in a fwe more tests (FJS and Nightwatch)
    • c8c4e032 - Add a temporary link under Structure in the admin menu
    • 0a92e3e5 - Tweak text for the temporary link in admin menu

    Compare with previous version

  • Benji Fisher added 72 commits

    added 72 commits

    • 0a92e3e5...a0db6dfb - 50 commits from branch project:10.1.x
    • a0db6dfb...ccf7d1ab - 12 earlier commits
    • e173fd1b - Make the helper function protected
    • 334e4a54 - Update test to match consolidated redirect function
    • e28cf4c8 - Test the status message, not the whole page text
    • 4e79017e - Test the query string
    • c94c40b8 - Give the testbot a break
    • caf7b175 - Revert "Give the testbot a break"
    • 6ed067f5 - Update paths in a fwe more tests (FJS and Nightwatch)
    • 3cdfab05 - Add a temporary link under Structure in the admin menu
    • 8e9bd314 - Tweak text for the temporary link in admin menu
    • f1069e9d - Clarify that the link, not the page, is legacy

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading