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

Conversation

alli83
Copy link
Collaborator

@alli83 alli83 commented Nov 18, 2024

Pull request for issue: #2033

This is a pull request for the following functionalities:

create a mockup for all upload status exception published
Also, save the dataset before creating the mockup

How to test?

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.

How have functionalities been implemented?

call submit action before calling create/reset url

Any issues with implementation?

Any changes to automated tests?

test added

Any changes to documentation?

Any technical debt repayment?

Any improvements to CI/CD pipeline?

Comment on lines 138 to 143
@ok
Scenario: Can't see create/reset private url
When I am on "/adminDataset/update/id/8"
Then I should not see "Create/Reset Private URL"

Copy link
Contributor

Choose a reason for hiding this comment

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

question: This scenario is also passing when I executed it from the develop branch, so I was wondering what is the purpose of this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's just a rearrangement of tests. Since dataset 8 is already published, we might as well reuse it directly.

@alli83 alli83 force-pushed the fix-2033-create-a-mockup-for-all-upload-statuses-except-published branch from 9ee2a8d to 46b51a5 Compare December 1, 2024 21:24
Comment on lines +475 to +488
@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."
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

@@ -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

Comment on lines +646 to +671
$(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')
}
})
});
});

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.

@alli83 alli83 marked this pull request as draft December 9, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants