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

fix: create a mockup for all upload status exception published #2093

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## Unreleased

- Fix #2033: Create a mockup for all upload statuses except published
- Feat #2066: Update file attribute values form layout and add expand button for long values
- Feat #2102: Delete outdated apidocs files
- Feat #1667: Add log entry when minting DOI
Expand Down
20 changes: 15 additions & 5 deletions protected/controllers/AdminDatasetController.php
Original file line number Diff line number Diff line change
Expand Up @@ -327,16 +327,26 @@ public function actionUpdate($id)
*/
public function actionPrivate()
{
$id = $_GET['identifier'];
$id = Yii::$app->request->get('identifier');
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Using Yii's built-in request handling, which is generally safer and more consistent

$model= Dataset::model()->find("identifier=?", array($id));
$datasetPageSettings = new DatasetPageSettings($model);
if ( "invalid" === $datasetPageSettings->getPageType() ) {
$pageType = $datasetPageSettings->getPageType();

if (!in_array($pageType, ['invalid', 'public', 'hidden', 'draft', 'mockup'])) {
throw new CHttpException(404, 'Page type not found');
}

if ( "invalid" === $pageType) {
$this->redirect('/site/index');
} elseif ( "public" === $datasetPageSettings->getPageType() ) {
} elseif ( "public" === $pageType) {
$this->redirect('/dataset/'.$model->identifier);
} elseif ( "hidden" === $datasetPageSettings->getPageType() || "draft" === $datasetPageSettings->getPageType() ) {
} else {
$model->token = Yii::$app->security->generateRandomString(16);
$model->save();

if (!$model->save()) {
throw new CHttpException(500, 'Fail to update dataset token');
}

$this->redirect('/dataset/'.$model->identifier.'/token/'.$model->token);
}
}
Expand Down
30 changes: 28 additions & 2 deletions protected/views/adminDataset/_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<?php } ?>

<?php if ($flashError = Yii::app()->user->getFlash('updateError')) { ?>
<div class="alert alert-danger" role="alert">
<div id="flashError" class="alert alert-danger" role="alert">
<?= $flashError ?>
</div>
<?php } ?>
Expand Down Expand Up @@ -554,7 +554,7 @@

if ($showCreateResetUrlBtn) {
?>
<a class="btn background-btn-o" href="<?php echo Yii::app()->createUrl('/adminDataset/private/identifier/' . $model->identifier) ?>" title="This will save any changes made on this page AND create a new mockup page URL/token link" data-toggle="tooltip">Create/Reset Private URL</a>
<a id="mockup" class="btn background-btn-o" href="<?php echo Yii::app()->createUrl('/adminDataset/private/identifier/' . $model->identifier) ?>" title="This will save any changes made on this page AND create a new mockup page URL/token link" data-toggle="tooltip">Create/Reset Private URL</a>
<?php
}
if ($showMockupBtn) {
Expand Down Expand Up @@ -643,6 +643,32 @@ class="form-control"
</div><!-- /.modal-dialog -->
</div><!-- /.modal -->
<script>
$(document).ready(function () {
$('#mockup').on('click', function (event) {
event.preventDefault();

mockupUrl = $(this).attr("href");
form = $('#dataset-form');

$.ajax({
url: form.attr('action'),
method: 'POST',
data: form.serialize(),
success: function (response, textStatus, xhr) {
if (200 === xhr.status) {
window.location.href = mockupUrl;
} else {
$('#flashError').html('An error occurred while trying to save the dataset')
}

},
error: function (error) {
$('#flashError').html('An error occurred while trying to save the dataset')
}
})
});
});

Comment on lines +646 to +671
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: This JS function/AJAX call makes sure to save any changes made in the form when clicking the Create/Reset Private URL, without refreshing or leaving the page. This can prevent loss of data and provide immediate feedback.
praise: Redirecting to the mockup page only after confirming the data in the form has been saved successfully.

$(function() {

var publication_date = $('.js-date-pub');
Expand Down
25 changes: 19 additions & 6 deletions tests/acceptance/AdminDatasetForm.feature
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,16 @@ Feature: form to update dataset details
Scenario: Can create/reset private url
When I am on "/adminDataset/update/id/5"
And I press the button "Create/Reset Private URL"
And I wait "1" seconds
And I wait "3" seconds
Then I should see current url contains "/dataset/100039/token/"
And I should see "Genomic data of the Puerto Rican Parrot (Amazona vittata) from a locally funded project."

@ok @dataset-status
Scenario: Can't see create/reset private url for a published dataset
When I am on "/adminDataset/update/id/8"
Then I should not see "Create/Reset Private URL"
And I should not see "Open Private URL"

@ok @issue-1023
Scenario: Open private url is working
When I am on "/adminDataset/update/id/5"
Expand Down Expand Up @@ -189,6 +195,7 @@ Feature: form to update dataset details
And I fill in the field of "name" "Dataset[title]" with "test dataset"
And I fill in the field of "name" "Dataset[identifier]" with "123789"
And I fill in the field of "name" "Dataset[ftp_site]" with "ftp://test"
When I check the field "Dataset_Epigenomic"
And I press the button "Create"
And I wait "1" seconds
And I am on "/adminDataset/update/id/2741"
Expand Down Expand Up @@ -465,14 +472,20 @@ Feature: form to update dataset details
| "DataAvailableForReview" |
| "DataPending" |

@ok @dataset-status
Scenario: Links to create mockup or to open mockup are not present for a published dataset
@ok @test
Scenario: Links to create mockup is present for a submitted dataset
Given I am on "/adminDataset/update/id/5"
And I select "Published" from the field "Dataset_upload_status"
And I select "DataAvailableForReview" from the field "Dataset_upload_status"
And I press the button "Save"
When I am on "/adminDataset/update/id/5"
Then I should not see "Create/Reset Private URL"
And I should not see "Open Private URL"
And I select "Submitted" from the field "Dataset_upload_status"
And I press the button "Save"
When I am on "/adminDataset/update/id/5"
Then I should see "Create/Reset Private URL"
And I press the button "Create/Reset Private URL"
And I wait "3" seconds
Then I should see current url contains "/dataset/100039/token/"
And I should see "Genomic data of the Puerto Rican Parrot (Amazona vittata) from a locally funded project."
Comment on lines +475 to +488
Copy link
Contributor

@kencho51 kencho51 Dec 2, 2024

Choose a reason for hiding this comment

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

question: Is this test still work in progress? Because it has tag @test. And is this test necessary as supposedly the following scenario should be implemented:

Given I am on the dataset Admin page of any dataset with a status of anything EXCEPT "Published"
When I look at the bottom of the page
Then I should see a create/reset private URL button
And when I click that button it does create or reset the private URL for that dataset.

But in fact, the current existing scenario Links to create mockup or to open mockup are always present for a non-published dataset has covered it already.
suggestion: It seems that this PR will not bring in any changes from the UI, so I was wondering is it better to test the JS function if it is testable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tag will be removed, just used locally.
the problem was with the submitted status (there was an error)
The published status is already tested. I just swapped the tests

Copy link
Contributor

@kencho51 kencho51 Dec 4, 2024

Choose a reason for hiding this comment

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

the problem was with the submitted status (there was an error)

If that is the case, may be the test can be:

Scenario: Cannot change status to Submitted from non DataAvailableForReview
 Given I am on "/adminDataset/update/id/668"
  And should see "Private"
  And I should see "Create/Reset Private URL"
  And I select `"Submitted"` from the field "Dataset_upload_status"
  When I press the button "Save"
  Then I am on "/adminDataset/update/id/668"
  And I should see "Failed to update status!"

Scenario: Links to create mockup is present for a submitted dataset
   Given I am on "/adminDataset/update/id/5"
    And I should see "ImportFromEM"
    And I select "DataAvailableForReview" from the field "Dataset_upload_status"
    And I press the button "Save"
    And I should see "Updated successfully! "
    And I should see "Create/Reset Private URL"
    When I select "Submitted" from the field "Dataset_upload_status"
    And I press the button "Save"
    And  I am on "/adminDataset/update/id/5"
    And I should see "Create/Reset Private URL"
    And I press the button "Create/Reset Private URL"
    And I wait "3" seconds
    Then I should see current url contains "/dataset/100039/token/"
    And I should see "Genomic data of the Puerto Rican Parrot (Amazona vittata) from a locally funded project."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

why do you want to add "Cannot change status to Submitted from non DataAvailableForReview". it's another PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remind me which PR? If there is already a PR for it, you can ignore Cannot change status to Submitted from non DataAvailableForReview"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not opened yet



@ok @issue-1812 @mockup
Expand Down