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

OPENEUROPA-1500: Create configuration UI for Webtools Analytics module #24

Merged
merged 11 commits into from
Jan 11, 2019

Conversation

nagyad
Copy link
Member

@nagyad nagyad commented Jan 4, 2019

OPENEUROPA-1500

Provide the Webtools Analytics with a UI so that users with appropriate permissions can edit and change webtools configuration.

@nagyad nagyad changed the title OPENEUROPA-1500 Create configuration UI for Webtools Analytics module OPENEUROPA-1500: Create configuration UI for Webtools Analytics module Jan 4, 2019
upchuk
upchuk previously approved these changes Jan 4, 2019
Copy link
Contributor

@richardcanoe richardcanoe left a comment

Choose a reason for hiding this comment

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

SiteID must be an integer. Probably worth adding either validation or at least a comment on the form.
Changes to the form are only picked up after the cache is cleared. Again, worth adding a cache clear as part of the form submission.
The test should be extended to check that the form fields are being used.

@nagyad
Copy link
Member Author

nagyad commented Jan 7, 2019

I have implemented the requested changes. The cache is going to be worked on with a followup ticket since the current solution requires some improvements.

@nagyad
Copy link
Member Author

nagyad commented Jan 7, 2019

I will also implement a custom permission for this.

richardcanoe
richardcanoe previously approved these changes Jan 8, 2019
richardcanoe
richardcanoe previously approved these changes Jan 8, 2019
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

👍 now

drupol
drupol previously approved these changes Jan 8, 2019
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

👍 all good now!

upchuk
upchuk previously approved these changes Jan 10, 2019
upchuk
upchuk previously approved these changes Jan 10, 2019
@BackupAnalyticsConfigs
Scenario: Create Webtools Analytics settings
Given I am logged in as a user with the "administer webtools analytics" permission
And I am on "admin/config/system/oe_webtools_analytics"
Copy link
Member

Choose a reason for hiding this comment

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

We should use the transformation context to turn this path into a business readable string, such as:

    And I am on "the Webtools Analytics configuration page"

And then in the behat.yml.dist:

      - OpenEuropa\Behat\TransformationContext:
          pages:
            Webtools Analytics configuration: "/admin/config/system/oe_webtools_analytics"

Copy link
Member Author

Choose a reason for hiding this comment

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

This does not work for me.

/**
* Name of the config being edited.
*/
const CONFIGNAME = 'oe_webtools_analytics.settings';
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename this into WebtoolsAnalyticsSettingsForm::CONFIG_NAME.

upchuk
upchuk previously approved these changes Jan 11, 2019
@nagyad nagyad merged commit d580d2b into master Jan 11, 2019
@nagyad nagyad deleted the OPENEUROPA-1500 branch January 11, 2019 09:33
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.

None yet

5 participants