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

Rename folder and file in Storage test #2214

Merged
merged 9 commits into from
Jul 8, 2022

Conversation

juans-chainsafe
Copy link
Contributor

closes #2204

@juans-chainsafe juans-chainsafe added the Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. label Jul 5, 2022
@juans-chainsafe juans-chainsafe self-assigned this Jul 5, 2022
@juans-chainsafe juans-chainsafe linked an issue Jul 5, 2022 that may be closed by this pull request
@render
Copy link

render bot commented Jul 5, 2022

@render
Copy link

render bot commented Jul 5, 2022

@render
Copy link

render bot commented Jul 5, 2022

import { basePage } from "./basePage"
import { createBucketModal } from "./modals/createBucketModal"

type BucketType = "ipfs" | "chainsafe"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

improvement suggested by Thibaut in #2207 (comment)

@juans-chainsafe juans-chainsafe changed the title Mnt/rename folder and file 2204 test Rename folder and file in Storage test Jul 5, 2022
// ensure the original name of the folder persists if the rename submission is blank
bucketContentsPage.renameFileOrFolder("{selectall}{del}{esc}")
bucketContentsPage.fileRenameInput().should("not.exist")
bucketContentsPage.fileItemName().contains(newFolderName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Original name has already been updated to the new one by this point (see assertion) so just to make the test read better then probably makes sense to swap the order of this check to before the rename, what do you think?

// ensure that the name of the folder is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.fileRenameInput().should("not.exist")
bucketContentsPage.fileItemKebabButton().first().click()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think first is necessary as there is only one folder present?

Suggested change
bucketContentsPage.fileItemKebabButton().first().click()
bucketContentsPage.fileItemKebabButton().click()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix this ones and the {selectall}{del} error in Files as well

bucketContentsPage.fileItemName().contains(newFolderName)

// ensure that the name of the folder is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.renameFileOrFolder("{selectall}{del}abc{esc}")

bucketContentsPage.fileItemName().contains(newFolderName)

// ensure that the name of the folder is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.renameFileOrFolder("{selectall}{del}abc{esc}")

bucketContentsPage.fileItemName().contains(newFileName)

// ensure that the name of the file is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.renameFileOrFolder("{selectall}{del}abc{esc}")

bucketContentsPage.fileItemName().contains(newFileName)

// ensure that the name of the file is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.renameFileOrFolder("{selectall}{del}abc{esc}")

},

renameFileOrFolder(newName: string) {
bucketContentsPage.fileItemKebabButton().first().click()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.fileItemKebabButton().first().click()
bucketContentsPage.fileItemKebabButton().click()

// ensure that the name of the folder is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.fileRenameInput().should("not.exist")
bucketContentsPage.fileItemKebabButton().first().click()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.fileItemKebabButton().first().click()
bucketContentsPage.fileItemKebabButton().click()

// ensure that the name of the file is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.fileRenameInput().should("not.exist")
bucketContentsPage.fileItemKebabButton().first().click()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.fileItemKebabButton().first().click()
bucketContentsPage.fileItemKebabButton().click()

// ensure that the name of the file is reset when renaming is canceled
bucketContentsPage.renameFileOrFolder("{selectall}abc{del}{esc}")
bucketContentsPage.fileRenameInput().should("not.exist")
bucketContentsPage.fileItemKebabButton().first().click()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bucketContentsPage.fileItemKebabButton().first().click()
bucketContentsPage.fileItemKebabButton().click()

cy.wait("@refresh")
},

createBucket(bucketName: string, bucketType: BucketType) {
Copy link
Contributor

@asnaith asnaith Jul 7, 2022

Choose a reason for hiding this comment

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

You could do this

Suggested change
createBucket(bucketName: string, bucketType: BucketType) {
createBucket(bucketName: string, bucketType: "ipfs" | "chainsafe") {

and then no need to have type BucketType = "ipfs" | "chainsafe" declared towards the top of the file.

As there are only two types of bucket you could also get rid of the switch for a simpler if / else statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sense to have it as a parameter because is only used there! and... yeah as well make sense that can be solved with if/else because is only 2 types of file system

Copy link
Contributor

@asnaith asnaith left a comment

Choose a reason for hiding this comment

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

Looks good @juans-chainsafe! Just left some minor suggestions.

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Great one and great review by Andrew, nothing else to add :)

@juans-chainsafe juans-chainsafe requested a review from asnaith July 7, 2022 13:27
@juans-chainsafe juans-chainsafe merged commit e3a8c2e into dev Jul 8, 2022
@juans-chainsafe juans-chainsafe deleted the mnt/rename-folder-and-file-2204-test branch July 8, 2022 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Storage] Rename folder and file test
3 participants