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

Don't show page fallback text if there is no alternative translation #1417

Merged
merged 1 commit into from May 11, 2022

Conversation

david-venhoff
Copy link
Member

Short description

This pr prevents the fallback text for pages to be shown when the alternatives linklist would be empty.
This should fix a bug where the fallback text was shown for empty pages which are only used for the tree structure.

@david-venhoff david-venhoff requested a review from a team as a code owner May 11, 2022 15:09
@codeclimate
Copy link

codeclimate bot commented May 11, 2022

Code Climate has analyzed commit 1c3e1e0 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 73.5% (0.0% change).

View more on Code Climate.

@svenseeberg
Copy link
Member

svenseeberg commented May 11, 2022

@david-venhoff thanks a lot! Would you be willing to add a test case which returns the fall back text? AFAICT no test case currently returns the fall back translation. That would probably catch most related problems in the future.

@david-venhoff david-venhoff force-pushed the bugfix/empty_fallback_text_linklist branch from ec14908 to b7a7103 Compare May 11, 2022 15:29
@david-venhoff
Copy link
Member Author

I've added a test case now (The fallback text is at line 426)

Copy link
Member

@svenseeberg svenseeberg left a comment

Choose a reason for hiding this comment

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

Thanks a lot (again)! The change is minor, the test case added. Looks good to me ;)

@svenseeberg
Copy link
Member

@timoludwig should we add an entry to the changelog?

@timobrembeck
Copy link
Member

@timoludwig should we add an entry to the changelog?

Yes, sounds good 👍

@svenseeberg
Copy link
Member

svenseeberg commented May 11, 2022

@david-venhoff as we do not have an issue for this, you can directly link to this PR. However, the normal link will mess with the build pipeline. But https://github.com/digitalfabrik/integreat-cms/issues/1417 does work as well as a link to PRs.

@david-venhoff david-venhoff force-pushed the bugfix/empty_fallback_text_linklist branch from b7a7103 to 1c3e1e0 Compare May 11, 2022 17:10
@david-venhoff david-venhoff merged commit e9cf59a into develop May 11, 2022
@david-venhoff david-venhoff deleted the bugfix/empty_fallback_text_linklist branch May 11, 2022 17:17
@timobrembeck timobrembeck mentioned this pull request May 22, 2022
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.

None yet

3 participants