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

Test/bulk cancel translation process #3117

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

theresantonie
Copy link
Contributor

@theresantonie theresantonie commented Oct 6, 2024

Short description

New test module to test the functionality of the CancelTranslationProcess class in page_bulk_actions.py.

Proposed changes

Add this test module to the general testing process and the needed test data to test_data.json:
pages: [30, 31] (Region: nurnberg)
pagetranslations: [100,101]

Side effects

Changed expected results for other tests:
sitemap_nurnberg_de.xml
num_expected_queries for nurnberg_de 78 -> 88

Resolved issues

Fixes: #2915


Pull Request Review Guidelines

Copy link

codeclimate bot commented Oct 8, 2024

Code Climate has analyzed commit 55238ca and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.5% (0.0% change).

View more on Code Climate.

@theresantonie theresantonie marked this pull request as ready for review October 9, 2024 07:56
Copy link
Member

@david-venhoff david-venhoff left a comment

Choose a reason for hiding this comment

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

Thanks, this looks mostly good to me.

Could you also verify that the a message is shown that the page is not in translation if the page was not in translation before, and that a message is shown that the status was successfully cancelled if the page was in translation before?

For reference, you could do something like that: https://github.com/digitalfabrik/integreat-cms/blob/develop/tests/cms/views/feedback/test_region_feedback_actions.py#L55-L59

integreat_cms/cms/fixtures/test_data.json Outdated Show resolved Hide resolved
integreat_cms/cms/fixtures/test_data.json Outdated Show resolved Hide resolved
@theresantonie
Copy link
Contributor Author

@david-venhoff
Thank you for the suggestion regarding the log messages.
I will implement it accordingly.

Regarding the archived pages:
There were some problems with other tests not passing, because the new pages were pulled into those tests and messed with the expected results.
There are many necessary changes and I haven't found a solution to some of them, yet.

@theresantonie
Copy link
Contributor Author

@david-venhoff
I have added the checks for the log messages.
During my tests, the messages were in German, so I had to add support for English and German log messages.

I also un-archived the pages and moved them to the Nürnberg region to reduce the impact on other tests.
This worked well, but the Editor and Management roles don't have the necessary rights in the Nürnberg region,
this would have to be added in the test_data.json. (But the side effects here are unclear to me, so far.)

@MizukiTemma
Copy link
Member

MizukiTemma commented Oct 30, 2024

@theresantonie
We can specify the language of error/success messages. See those lines
(settings: SettingsWrapper must be then included into the parameters of the test function)

@theresantonie
Copy link
Contributor Author

@MizukiTemma
Thank you for the hint. I was already wondering.

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.

Add test for translation cancel bulk action
3 participants