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

Remove max from row limit setting, combine forms #3693

Merged
merged 6 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 12 additions & 15 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
/vendor
/core
/node_modules
/web
/libraries
/themes/infinite/
.DS_Store
composer.lock
/tests/reports
.env.local
.swp
/docroot
.docksal
.DS_Store
.idea
/docs
.swp
.vscode
coverage
cypress/videos
composer.lock
package-lock.json
tagfile
/docs
/core
/cypress/videos
/cypress/screenshots
/libraries
/node_modules
/vendor
/tests/reports
/web
8 changes: 4 additions & 4 deletions cypress/integration/09_admin_links.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ context('Administration pages', () => {
cy.get('.option').should('contain.text', 'Distribution (distribution)')
})

it('I should see a link for the SQL endpoint configuration', () => {
it('I should see a link for the datastore configuration', () => {
cy.visit(baseurl + "/admin")
cy.get('.toolbar-icon-system-admin-dkan').contains('DKAN').next('.toolbar-menu').then($el=>{
cy.wrap($el).invoke('show')
cy.wrap($el).contains('SQL endpoint')
cy.wrap($el).contains('Datastore settings')
})
cy.visit(baseurl + "/admin/dkan/sql-endpoint")
cy.get('label').should('have.text', 'Rows limit')
cy.visit(baseurl + "/admin/dkan/datastore")
cy.get('label[for="edit-rows-limit"]').should('have.text', 'Rows limit')
})
})
18 changes: 6 additions & 12 deletions modules/datastore/datastore.links.menu.yml
Original file line number Diff line number Diff line change
@@ -1,26 +1,20 @@
datastore.settings_form:
title: SQL endpoint
route_name: datastore.sql_endpoint.settings
description: Configure SQL query API endpoint.
title: Datastore settings
description: Datastore settings.
parent: system.admin_dkan
route_name: datastore.settings
weight: 10

datastore.dkan_resource_settings:
title: Resources
description: Resource options.
parent: system.admin_dkan
route_name: datastore.dkan_resource_settings
weight: 10
weight: 11

datastore.datasets_import_status_dashboard:
title: 'Datastore Import Status'
parent: system.admin_dkan
description: 'View status information on DKAN datastore imports.'
route_name: datastore.datasets_import_status_dashboard
weight: 10

datastore.triggering_settings:
title: Datastore settings
description: Datastore settings.
parent: system.admin_dkan
route_name: datastore.settings
weight: 10
weight: 12
2 changes: 1 addition & 1 deletion modules/datastore/src/Controller/QueryController.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class QueryController implements ContainerInjectionInterface {
*
* @var int
*/
protected const DEFAULT_ROWS_LIMIT = 500;
public const DEFAULT_ROWS_LIMIT = 500;

/**
* Api constructor.
Expand Down
21 changes: 15 additions & 6 deletions modules/datastore/src/Form/DatastoreSettingsForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@

use Drupal\Core\Form\ConfigFormBase;
use Drupal\Core\Form\FormStateInterface;
use Drupal\datastore\Controller\QueryController;
use Drupal\metastore\SchemaPropertiesHelper;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* Class DatastoreSettingsForm.
* Datastore settings form.
*
* @package Drupal\datastore\Form
* @codeCoverageIgnore
Expand Down Expand Up @@ -59,14 +60,21 @@ protected function getEditableConfigNames() {
* {@inheritdoc}
*/
public function buildForm(array $form, FormStateInterface $form_state) {
$form['fieldset'] = [
'#type' => 'fieldset',
'#title' => $this->t('Triggering properties'),
'#description' => $this->t('Property to trigger update of the datastore.'),
$form['rows_limit'] = [
'#type' => 'number',
'#min' => 1,
'#title' => $this->t('Rows limit'),
'#default_value' => $this->config('datastore.settings')->get('rows_limit'),
'#description' => $this->t('Maximum number of rows the datastore endpoints can return
in a single request. Caution: setting too high can lead to timeouts or memory issues.
Default 500; values above 20,000 not recommended.'),
];
$form['fieldset']['triggering_properties'] = [

$form['triggering_properties'] = [
'#type' => 'checkboxes',
'#title' => $this->t('Datastore triggering properties'),
'#description' => $this->t('Metadata properties whose change will trigger a re-import of
an associated resource to the datastore.'),
'#options' => $this->schemaHelper->retrieveSchemaProperties('dataset'),
'#default_value' => $this->config('datastore.settings')->get('triggering_properties'),
];
Expand All @@ -78,6 +86,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
*/
public function submitForm(array &$form, FormStateInterface $form_state) {
$this->config('datastore.settings')
->set('rows_limit', $form_state->getValue('rows_limit') ?: QueryController::DEFAULT_ROWS_LIMIT)
->set('triggering_properties', $form_state->getValue('triggering_properties'))
->save();
parent::submitForm($form, $form_state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
/**
* Settings form for the SQL endpoint.
*
* NO LONGER USED; TO BE REMOVED.
*
* @package Drupal\sql_endpoint\Form
* @todo Remove
*
* @codeCoverageIgnore
* @todo Add test coverage
*/
class DkanSqlEndpointSettingsForm extends ConfigFormBase {

Expand Down