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

AV-2034: Fix header search breaking on consecutive searches #2037

Merged
merged 9 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@
[submodule "modules/ckanext-sixodp_showcasesubmit"]
path = ckan/ckanext/ckanext-sixodp_showcasesubmit
url = https://github.com/vrk-kpa/ckanext-sixodp_showcasesubmit.git
[submodule "modules/ckanext-datarequests"]
path = ckan/ckanext/ckanext-datarequests
url = https://github.com/vrk-kpa/ckanext-datarequests.git
[submodule "modules/ckanext-organizationapproval"]
path = ckan/ckanext/ckanext-organizationapproval
url = https://github.com/vrk-kpa/ckanext-organizationapproval.git
Expand Down
1 change: 0 additions & 1 deletion cdk/lib/ckan-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ export class CkanStack extends Stack {
'dcat_json_interface',
'csw_harvester',
'drupal8',
//'datarequests',
'ytp_organizations',
'ytp_request',
'hierarchy_display',
Expand Down
1 change: 0 additions & 1 deletion ckan/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@ COPY ckanext/ckanext-ytp_request ${EXT_DIR}/ckanext-ytp_request/
COPY ckanext/ckanext-ytp_main ${EXT_DIR}/ckanext-ytp_main/
COPY ckanext/ckanext-hierarchy ${EXT_DIR}/ckanext-hierarchy/
COPY ckanext/ckanext-matomo ${EXT_DIR}/ckanext-matomo/
COPY ckanext/ckanext-datarequests ${EXT_DIR}/ckanext-datarequests/
COPY ckanext/ckanext-harvest ${EXT_DIR}/ckanext-harvest/
COPY ckanext/ckanext-report ${EXT_DIR}/ckanext-report/
COPY ckanext/ckanext-spatial ${EXT_DIR}/ckanext-spatial/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ def search():
query = get_action('package_search')(context, data_dict)

json_query = json.dumps(
{k: v for k, v in list(params_to_dict(request.form).items()) if k != 'page' and type(v) is list and len(v[0]) > 0}
{k: v for k, v in list(params_to_dict(request.form).items()) if k != 'page' and isinstance(v, list) and len(v[0]) > 0}
)

filters = {
k: v for k, v in list(params_to_dict(request.form).items()) if k != 'search_target' and k != 'search_query'
and k != 'page' and k != 'released-before' and k != 'released-after' and k != 'updated-before'
and k != 'updated-after' and k != 'sort' and type(v) is list and len(v[0]) > 0
and k != 'updated-after' and k != 'sort' and isinstance(v, list) and len(v[0]) > 0
}

for key, value in filters.items():
Expand Down
1 change: 0 additions & 1 deletion ckan/ckanext/ckanext-datarequests
Submodule ckanext-datarequests deleted from 0076e2
4 changes: 2 additions & 2 deletions ckan/ckanext/ckanext-ytp_main/ckanext/ytp/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,13 @@
resource['description_translated'] = {
original_language: resource.get('description', '') or ''
}
if resource.get('temporal_granularity') and type(resource.get('temporal_granularity')) is not dict:
if resource.get('temporal_granularity') and not isinstance(resource.get('temporal_granularity'), dict):

Check warning on line 199 in ckan/ckanext/ckanext-ytp_main/ckanext/ytp/cli.py

View check run for this annotation

Codecov / codecov/patch

ckan/ckanext/ckanext-ytp_main/ckanext/ytp/cli.py#L199

Added line #L199 was not covered by tests
resource['temporal_granularity'] = {
original_language: resource.get('temporal_granularity')
}
else:
del resource['temporal_granularity']
if resource.get('update_frequency') and type(resource.get('update_frequency')) is not dict:
if resource.get('update_frequency') and not isinstance(resource.get('update_frequency'), dict):

Check warning on line 205 in ckan/ckanext/ckanext-ytp_main/ckanext/ytp/cli.py

View check run for this annotation

Codecov / codecov/patch

ckan/ckanext/ckanext-ytp_main/ckanext/ytp/cli.py#L205

Added line #L205 was not covered by tests
resource['update_frequency'] = {
original_language: resource.get('update_frequency')
}
Expand Down
6 changes: 3 additions & 3 deletions ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@


def as_dict(value):
if type(value) is dict:
if isinstance(value, dict):

Check warning on line 37 in ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py

View check run for this annotation

Codecov / codecov/patch

ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py#L37

Added line #L37 was not covered by tests
return value
elif type(value) in (str, six.text_type):
elif isinstance(value, str) or isinstance(value, six.text_type):

Check warning on line 39 in ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py

View check run for this annotation

Codecov / codecov/patch

ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py#L39

Added line #L39 was not covered by tests
return as_dict(json.loads(value))
else:
raise ValueError()
Expand Down Expand Up @@ -507,7 +507,7 @@
if responsible_party:
try:
custodians_data = json.loads(responsible_party)
custodians_data = custodians_data if type(custodians_data) is list else [custodians_data]
custodians_data = custodians_data if isinstance(custodians_data, list) else [custodians_data]

Check warning on line 510 in ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py

View check run for this annotation

Codecov / codecov/patch

ckan/ckanext/ckanext-ytp_main/ckanext/ytp/dcat.py#L510

Added line #L510 was not covered by tests
for custodian_data in custodians_data:
custodian = BNode()
g.add((custodian, RDF.type, FOAF.Agent))
Expand Down
2 changes: 1 addition & 1 deletion ckan/ckanext/ckanext-ytp_main/ckanext/ytp/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ def before_index(self, pkg_dict):
prop_value = json.loads(prop_json)
# Add for each language
for lang in languages:
if type(prop_value) is dict and prop_value.get(lang):
if isinstance(prop_value, dict) and prop_value.get(lang):
prop_value[lang] = [tag for tag in {tag.lower() for tag in prop_value[lang]} if tag not in ignored_tags]
pkg_dict['vocab_%s_%s' % (prop_key, lang)] = [tag for tag in prop_value[lang]]
pkg_dict[prop_key] = json.dumps(prop_value)
Expand Down
4 changes: 0 additions & 4 deletions ckan/scripts/install_extensions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ pip_install "${EXT_DIR}/ckanext-ytp_request/requirements.txt"
pip_install "${EXT_DIR}/ckanext-ytp_main/requirements.txt"
pip_install "${EXT_DIR}/ckanext-hierarchy/requirements.txt"
pip_install "${EXT_DIR}/ckanext-matomo/requirements.txt"
pip_install "${EXT_DIR}/ckanext-datarequests/requirements.txt"
pip_install "${EXT_DIR}/ckanext-harvest/requirements.txt"
pip_install "${EXT_DIR}/ckanext-report/requirements.txt"
pip_install "${EXT_DIR}/ckanext-spatial/requirements.txt"
Expand Down Expand Up @@ -57,7 +56,6 @@ pip_install "${EXT_DIR}/ckanext-ytp_request/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-ytp_main/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-hierarchy/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-matomo/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-datarequests/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-harvest/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-report/pip-requirements.txt"
pip_install "${EXT_DIR}/ckanext-spatial/pip-requirements.txt"
Expand Down Expand Up @@ -92,7 +90,6 @@ pip install -e ${EXT_DIR}/ckanext-drupal8 \
-e ${EXT_DIR}/ckanext-ytp_main \
-e ${EXT_DIR}/ckanext-hierarchy \
-e ${EXT_DIR}/ckanext-matomo \
-e ${EXT_DIR}/ckanext-datarequests \
-e ${EXT_DIR}/ckanext-harvest \
-e ${EXT_DIR}/ckanext-report \
-e ${EXT_DIR}/ckanext-spatial \
Expand Down Expand Up @@ -130,7 +127,6 @@ pip install -e ${EXT_DIR}/ckanext-drupal8 \
(cd ${EXT_DIR}/ckanext-advancedsearch; python setup.py compile_catalog -f)
(cd ${EXT_DIR}/ckanext-scheming; python setup.py compile_catalog -f)
(cd ${EXT_DIR}/ckanext-statistics; python setup.py compile_catalog -f)
(cd ${EXT_DIR}/ckanext-datarequests; python setup.py compile_catalog -f)
(cd ${EXT_DIR}/ckanext-matomo; python setup.py compile_catalog -f)
(cd ${EXT_DIR}/ckanext-ytp_recommendation; python setup.py compile_catalog -f)
(cd ${EXT_DIR}/ckanext-apis; python setup.py compile_catalog -f)
Expand Down
2 changes: 0 additions & 2 deletions ckan/templates/production.ini.j2
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ ckanext.sixodp_showcasesubmit.recipient_emails = {{ environ('SYSADMIN_EMAIL') }}
ckanext.ytp_recommendation.recaptcha_sitekey = {{ environ('RECAPTCHA_PUBLIC_KEY') }}
ckanext.ytp_recommendation.recaptcha_secret = {{ environ('RECAPTCHA_PRIVATE_KEY') }}

ckanext-datarequests.datarequest_notifications_admin_email = None

ckanext.drupal8.site_url = {{ environ('CKAN_DRUPAL_SITE_URL') }}
ckanext.drupal8.domain = {{ environ('DOMAIN_NAME') }},{{ environ('SECONDARY_DOMAIN_NAME') }}
ckanext.drupal8.sysadmin_role = {{ environ('ROLES_CKAN_ADMIN') }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Drupal\Core\Controller\ControllerBase;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Drupal\Core\Url;

/**
* Adds support for header controller.
Expand All @@ -29,7 +30,8 @@ public function header() {
$queryParams = \Drupal::request()->query->all();

if (isset($queryParams['activePath'])) {
\Drupal::service('path.current')->setPath($queryParams['activePath']);
$url = Url::fromUserInput($queryParams['activePath']);
\Drupal::service('path.current')->setPath($url->toString());
}

$response = new Response(\Drupal::service('renderer')->renderRoot($build));
Expand Down
19 changes: 8 additions & 11 deletions drupal/modules/avoindata-header/src/Plugin/Form/SearchForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public function getFormId() {
* {@inheritdoc}
*/
public function buildForm(array $form, FormStateInterface $form_state) {
// AV-2034: Set form action explicitly to drop query parameters reliably.
$language = \Drupal::languageManager()->getCurrentLanguage()->getId();
$form['#action'] = sprintf('/%s/api/header', $language);

$form['searchfilter'] = [
'#type' => 'textfield',
Expand Down Expand Up @@ -56,19 +59,13 @@ public function buildForm(array $form, FormStateInterface $form_state) {
public function submitForm(array &$form, FormStateInterface $form_state) {
$filter = $form_state->getValue('searchfilter');
$language = \Drupal::languageManager()->getCurrentLanguage()->getId();
$base_path = '/data/%s';

if ($language === 'en') {
$base_path = sprintf($base_path, 'en_GB');
$language = 'en_GB';
}
else {
$base_path = sprintf($base_path, $language);
}

$base_path = $base_path . '/search';
$base_path = $base_path . '?q=%s';
$redirect_path = sprintf($base_path, $form_state->getValue('search'));
$url = url::fromUserInput($redirect_path);
$path = sprintf('/data/%s/search', $language);
$url = url::fromUserInput($path, [
'query' => ['q' => $form_state->getValue('search')],
]);
$form_state->setRedirectUrl($url);
}

Expand Down