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 broken translation status if only minor public versions exists #1854

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

JoeyStk
Copy link
Contributor

@JoeyStk JoeyStk commented Nov 10, 2022

Short description

Fixes the issue with the broken translation status if only minor public versions exists for POI and event

Proposed changes

  • Add extra if-condition to ensure the correct translation status is set

Side effects

I don't think there are any.

Resolved issues

Fixes: #1788


Pull Request Review Guidelines

@JoeyStk JoeyStk requested a review from a team as a code owner November 10, 2022 19:49
@JoeyStk JoeyStk force-pushed the bugfix/broken_translation_status branch from 3b20817 to c83744c Compare November 10, 2022 19:52
@codeclimate
Copy link

codeclimate bot commented Nov 10, 2022

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

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

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

View more on Code Climate.

Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Thank you for PR 👍

I think this is just a copy & paste thing though have to suggest a change 😅 See code suggestion.

integreat_cms/cms/views/events/event_form_view.py Outdated Show resolved Hide resolved
Copy link
Member

@timobrembeck timobrembeck 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, works as expected! 👍

CHANGELOG.md Outdated Show resolved Hide resolved
integreat_cms/cms/views/events/event_form_view.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/pois/poi_form_view.py Outdated Show resolved Hide resolved
@JoeyStk JoeyStk force-pushed the bugfix/broken_translation_status branch 2 times, most recently from 15a4e13 to 5264588 Compare November 17, 2022 09:15
@JoeyStk
Copy link
Contributor Author

JoeyStk commented Nov 17, 2022

Thank you :) All of it makes sense :)

CHANGELOG.md Outdated Show resolved Hide resolved
@JoeyStk JoeyStk force-pushed the bugfix/broken_translation_status branch from 5264588 to 67a2d9b Compare November 17, 2022 12:07
@timobrembeck timobrembeck mentioned this pull request Nov 18, 2022
@seluianova seluianova self-assigned this Nov 18, 2022
@seluianova seluianova force-pushed the bugfix/broken_translation_status branch from 7d7d97f to 8477414 Compare November 18, 2022 13:33
Copy link
Contributor

@seluianova seluianova left a comment

Choose a reason for hiding this comment

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

When DRAFT event/POI is published, translation status is correct now ✅

However when status is still DRAFT, it shows "translation missing". Is it an expected behavior or another issue?

@seluianova seluianova merged commit 783a05c into develop Nov 18, 2022
@seluianova seluianova deleted the bugfix/broken_translation_status branch November 18, 2022 14:36
@timobrembeck
Copy link
Member

When DRAFT event/POI is published, translation status is correct now ✅

However when status is still DRAFT, it shows "translation missing". Is it an expected behavior or another issue?

This was defined as expected behavior by the service team since all draft content is not visible in the app and is thus "missing".

@seluianova
Copy link
Contributor

@timoludwig looks a bit strange to me, because "missing" is more like "not exists". And to mark thing as "visible" there is a "published" status... But okay, thanks for the answer :)

@timobrembeck
Copy link
Member

@seluianova yes, I agree, this probably needs to be revisited in the future together with the service team and UI/UX.

@seluianova seluianova removed their assignment Jan 13, 2023
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.

Broken translation status if only minor public versions exist
4 participants