Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDPAP-7604] Add Redirect bulk import feature #404

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

meralvingrita24
Copy link
Contributor

@meralvingrita24 meralvingrita24 commented May 10, 2023

Jira

https://digital-vic.atlassian.net/browse/SDPAP-7604

Changes

  1. Install the path_redirect_import module via composer
  2. Patch the path_redirect_import module to improve validation
  3. Create a hook update script that will enable the newly added module.

Related issues

Screenshots

Screenshot 2023-05-10 at 12 47 40 PM

…cting

1. Initial commit for adding and installing path redirect import (https://www.drupal.org/project/issues/path_redirect_import) module for doing bulk url redirecting via CSV import
@christopher-hopper
Copy link
Contributor

@meralvingrita24

We should make a few changes to our Drupal issue queue patch.

Remove the if/else wrapping of existing validations and only add the check you want to happen before existing validations.

If you want to exit the validations early, then try using a continue; or an early return, rather than wrapping all existing validations in a new if/else statement block.

The aim is to keep our patch change as minimal as possible to achieve the result we want.

@christopher-hopper
Copy link
Contributor

@meralvingrita24 is there a place in this module, where we can add the example "CSV" file as a template for bulk import of URL Redirects.

Suggest adding some minimal documentation into the README file or somewhere appropriate.

@meralvingrita24
Copy link
Contributor Author

Hello @christopher-hopper , as discussed, the patch has been updated with a simplified condition to handle warning messages. As well, here is a screenshot of the instructions that has been in the module, which serves as a guide for CSV patterns.

Screenshot 2023-05-18 at 9 47 10 PM

Please let me know your thoughts here.

@christopher-hopper christopher-hopper changed the title SDPAP-7604 - Add and install Path Redirect Import for bulk URL Redirecting [SDPAP-7604] Add Redirect bulk import feature May 30, 2023
…rt-module

# Conflicts:
#	tide_core.install

Resolving tide_core.install conflict
@meralvingrita24
Copy link
Contributor Author

Resolved merge conflict from tide_core.install

@@ -1735,3 +1735,13 @@ function tide_core_update_8076() {
$config_storage = \Drupal::service('config.storage');
$config_storage->write($view, $source->read($view));
}

/*
Copy link
Contributor

@christopher-hopper christopher-hopper May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meralvingrita24
coding standards violation
pls fix

Don't forget to turn on PHPCS in your IDE with the Drupal standards at a minimum

It'll catch this stuff before you push your code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @christopher-hopper - good day. Regarding the coding standard issue, I raised a new PR to addressed it: 810b30b

Please check and review.

Copy link
Contributor

@christopher-hopper christopher-hopper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code reviewed. Ready for functional testing.

@github-actions github-actions bot added the Stale label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants