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

Feature/ADF-1695/Show multiple preview buttons #474

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

jsconan
Copy link
Contributor

@jsconan jsconan commented Jul 4, 2024

Related to: https://oat-sa.atlassian.net/browse/ADF-1695

Summary

Add the possibility of having more than one test previewer.

Details

It supports multiple preview buttons from the test's property page.

This is made thanks to a configuration applied in config/tao/client_lib_config_registry.conf.php, and a change applied to the structures.xml file.

  • structures.xml
    Each button must be declared with an identifier starting with test-preview. The first button must be test-preview, the second test-preview-1, etc. The suffix number refers to a position in the configuration (see client_lib_config_registry.conf.php).
    Example:
    <structure id="tests" name="Tests" level="1" group="main">
        <sections>
            <section id="manage_tests" name="Manage tests" url="/taoTests/Tests/index">
                <actions allowClassActions="true">
                    <action id="test-preview" name="Preview" url="/taoTests/Tests/index" context="instance" group="content" binding="testPreview">
                        <icon id="icon-preview"/>
                    </action>
                    <action id="test-preview-1" name="Preview (legacy)" url="/taoTests/Tests/index" context="instance" group="content" binding="testPreview">
                        <icon id="icon-preview"/>
                    </action>
                </actions>
            </section>
        </sections>
    </structure>
  • client_lib_config_registry.conf.php
    The configuration can be extended with a list of possible providers, aside from the default one. This is done in config/tao/client_lib_config_registry.conf.php.
    Example:
        'taoTests/controller/tests/action' => array(
            'provider' => 'qtiTestNUI',
            'providers' => array(
                array(
                    'id' => 'qtiTestNUI',
                    'label' => 'Preview'
                ),
                array(
                    'id' => 'qtiTest',
                    'label' => 'Preview (legacy)'
                )
            )
        ),

How to test

  • checkout the branch: git checkout -t origin/feature/ADF-1695/multiple-preview-buttons
  • also checkout the branches from the companion PR
  • make sure to have set the appropriate configuration (check the details in the ticket)

Copy link

github-actions bot commented Jul 4, 2024

Front-end summary Node 18

💯 Total ✅ Passed ⏭️ Skipped ❌ Failed
0 0 0 0

@jsconan jsconan requested review from gabrielfs7 and shaveko July 4, 2024 08:57

const getPreviewId = idx => `test-preview${idx ? `-${idx}` : ''}`;
const previewActions = []
for (let i = 0; i < 10; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see these logics are repeated in the item extension too. I've left some comments there that may apply here too (like, if the fixed 10 may become a problem later).

Would it be possible to have this duplicated logic centralized in some place instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, not the best. I've updated it with a constant instead.

Copy link

github-actions bot commented Jul 4, 2024

Version

Target Version 16.2.0
Last version 16.1.4

There are 0 BREAKING CHANGE, 2 features, 0 fix

@jsconan jsconan requested a review from gabrielfs7 July 4, 2024 10:48
Copy link
Contributor

@shaveko shaveko left a comment

Choose a reason for hiding this comment

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

Looks fine

Looks fine

  • New code is covered by tests (if applicable)
  • Tests are running successfully (old and new ones) on my local machine (if applicable)
  • New code is respecting code style rules
  • New code is respecting best practices
  • New code is not subject to concurrency issues (if applicable)
  • Feature is working correctly on my local machine (if applicable)
  • Acceptance criteria are respected
  • Pull request title and description are meaningful

Copy link
Contributor

@gabrielfs7 gabrielfs7 left a comment

Choose a reason for hiding this comment

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

LGTM

  • New code is covered by tests (if applicable)
  • Tests are running successfully (old and new ones) on my local machine (if applicable)
  • New code is respecting code style rules
  • New code is respecting best practices
  • New code is not subject to concurrency issues (if applicable)
  • Feature is working correctly on my local machine (if applicable)
  • Acceptance criteria are respected
  • Pull request title and description are meaningful

@jsconan jsconan merged commit 0b3aa0a into develop Jul 9, 2024
5 checks passed
@jsconan jsconan deleted the feature/ADF-1695/multiple-preview-buttons branch July 9, 2024 07:36
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.

3 participants