Problem/Motivation

Write functional tests for the modules functionality. We need to ensure it works as expected!

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork revisions_bulk_operations-3586317

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue tags: +Novice

Might be a nice novice task to learn to write tests in Drupal?

sapnil_biswas made their first commit to this issue’s fork.

sapnil_biswas opened merge request !3

sapnil_biswas’s picture

sapnil_biswas commented
Status: Active » Needs review

Added the functional test, marking this for review

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

Thanks @sapnil_biswas sadly they seem to fail?

Could you please also add permission tests (separate tests per permissions - testing with AND without each permission and in useful combinations)?
And a generic test like this please in addition: https://git.drupalcode.org/project/existing_values_autocomplete_widget/-...

Maybe you'd also like to take over #3586292: Fix cspell, phpcs, phpstan, eslint and make everything green required?

sapnil_biswas’s picture

sapnil_biswas commented

Sorry for the inconvenience, I will get them fixed and then start with the https://www.drupal.org/project/revisions_bulk_operations/issues/3586292

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@sapnil_biswas thanks, tests are still failing.

Furthermore I think not all permission conditions are tested, for example not being able to access the revisions at all if the user does not have 'view page revisions' permission? Even if this is more or less part of core, we should ensure that we don't break that permission in the module. Also check the other involved permission to ensure they are tested in all useful combinations.

Also please add a test to delete all revisions at once, currently only deleting 1 revision is tested, right?

Also please test selecting no revision and clicking the delete button to work as expected, NOT deleting any revision and showing an error message.

Thanks! We're close to the finish line :)

sapnil_biswas’s picture

sapnil_biswas commented

Thanks for the review and feedback, @anybody. Once i get the #3586292: Fix cspell, phpcs, phpstan, eslint and make everything green required I will this one fixed asap

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@sapnil_biswas the CI issue is now merged :) You can proceed.

sapnil_biswas’s picture

sapnil_biswas commented
Status: Needs work » Needs review

@anybody, it's green now. Please have a look when you have time

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil

@grevil please do the final review.

grevil’s picture

grevil commented
Assigned: grevil » anybody

Great tests, thanks! @anybody I made one comment for you.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: anybody » Unassigned
Status: Needs review » Needs work

Agreed! Here's the text, as similar in core:
"At least one revision must be selected."

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Please do it here, that's fine for me.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@sapnil_biswas thanks for your work here, now you know the module, you may also be able to resolve this one finally? #3581727: Improve the module page, README.md and add screenshots

sapnil_biswas’s picture

sapnil_biswas commented
Status: Needs work » Needs review

@anybody, I have updated the text as required. I will looking forward to #3581727 now

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil
grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs review » Reviewed & tested by the community

Great stuff, thanks @sapnil_biswas!

grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.