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

[SD-345] Implement Ability to Remove Quick Exit from Specific Site Sections #524

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

Conversation

sharmasahil
Copy link

@sharmasahil sharmasahil commented Oct 16, 2024

Jira

SD-345 - Implement Ability to Remove Quick Exit from Specific Site Sections

Changes

Added new field in sites taxonomy as 'Show Exit Site Specific?' only visible if its a sub-site.

@vincent-gao
Copy link
Contributor

vincent-gao commented Oct 16, 2024

hey @sharmasahil ,
thanks for the changes.
but please update the PR title to follow the format below and ensure it’s clear and meaningful.
e.g. [SD-345] Add Site-Specific “Quick Exit” Button Field to Sites Taxonomy

details of the PR

### Jira
https://digital-vic.atlassian.net/browse/SD-345

### Changes
.........

Copy link
Contributor

@vincent-gao vincent-gao left a comment

Choose a reason for hiding this comment

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

hey @sharmasahil please see my comments.

@sharmasahil
Copy link
Author

sharmasahil commented Oct 16, 2024

hey @sharmasahil , thanks for the changes. but please update the PR title to follow the format below and ensure it’s clear and meaningful. e.g. [SD-345] Add Site-Specific “Quick Exit” Button Field to Sites Taxonomy

details of the PR

### Jira
https://digital-vic.atlassian.net/browse/SD-345

### Changes
.........

Done

@vincent-gao
Copy link
Contributor

vincent-gao commented Oct 16, 2024

@sharmasahil the title~
CleanShot 2024-10-16 at 20 32 24
change it from Feature/sd 345 quick exit to [SD-345] Implement Ability to Remove Quick Exit from Specific Site Sections
I have changed it for you.

@vincent-gao vincent-gao changed the title Feature/sd 345 quick exit [SD-345] Implement Ability to Remove Quick Exit from Specific Site Sections Oct 16, 2024
@vincent-gao
Copy link
Contributor

hey @sharmasahil some issues,

  1. The tests have failed. Please review the details here https://github.com/dpc-sdp/tide_core/actions/runs/11360325150/job/31597880122?pr=524 .
  2. Please try to install the tide_core module locally and then enable the tide_site module. Once installed, run the behat tests for both tide_site and tide_core. For more details on setting up and running tests, refer to the development and maintenance guide.
    • The reason I’m requesting this is that we currently don’t have a way to automate testing for submodules. To be safe, please perform manual testing for now. The issue with automating these tests will be addressed in a future sprint.

@sharmasahil
Copy link
Author

hey @sharmasahil some issues,

  1. The tests have failed. Please review the details here https://github.com/dpc-sdp/tide_core/actions/runs/11360325150/job/31597880122?pr=524 .

  2. Please try to install the tide_core module locally and then enable the tide_site module. Once installed, run the behat tests for both tide_site and tide_core. For more details on setting up and running tests, refer to the development and maintenance guide.

    • The reason I’m requesting this is that we currently don’t have a way to automate testing for submodules. To be safe, please perform manual testing for now. The issue with automating these tests will be addressed in a future sprint.

This is fixed Vincent and also behat test require fix as discussed.

Copy link
Contributor

@MdNadimHossain MdNadimHossain left a comment

Choose a reason for hiding this comment

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

@seanhamlin thanks for making the changes. The only thing I am requesting is changing the var name in js for the parent field. Have a look. Thanks

* New field site specific quick exit button.
*/

function tide_site_update_10002() {
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong indention, It has once extra space

$update_service->revert($type, $configs_name);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong indention


Drupal.behaviors.siteQuickExitFields = {
attach: function (context, settings) {
var parent = ($('.taxonomy-term-sites-form #edit-parent'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using parent, which is a common jQuery method, use a more descriptive variable name like parentField to avoid confusion.

@sharmasahil
Copy link
Author

@seanhamlin thanks for making the changes. The only thing I am requesting is changing the var name in js for the parent field. Have a look. Thanks

@MdNadimHossain This is fixed Nadim, Thanks!!

@vincent-gao
Copy link
Contributor

hey @sharmasahil could you please fix the linting issues, more details https://github.com/dpc-sdp/tide_core/actions/runs/11397035111/job/31711837299?pr=524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants