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

Location endpoint raises internal server error when default translation is not public #2097

Closed
timobrembeck opened this issue Feb 23, 2023 · 2 comments · Fixed by #2125
Closed
Assignees
Labels
‼️ prio: high Needs to be resolved ASAP. 🐛 bug Something isn't working ☺️ effort: low Should be doable in <4h
Milestone

Comments

@timobrembeck
Copy link
Member

Describe the Bug

When a region creates a location which is a draft in the default language, but public in other languages, the location API returns an internal server error

Steps to Reproduce

  1. Create location as draft in default language
  2. Publish location in different language
  3. Access location API (e.g. https://admin.integreat-app.de/api/maintal/en/locations/)
  4. See error

Expected Behavior

  1. It should not be possible to publish a POI translation in a different language when the default translation is not public
  2. In case this inconsistent state is reached anyway, the API should not deliver those POIs.

Actual Behavior

The database inconsistency causes an internal server error

Additional Information

Traceback
Feb 23 15:52:35 ERROR   django.request - 500 Internal Server Error: /maintal/en/wp-json/extensions/v3/locations/?on_map=1
Traceback (most recent call last):
File "/opt/integreat-cms/.venv/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
  response = get_response(request)
File "/opt/integreat-cms/.venv/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
  response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/opt/integreat-cms/.venv/lib/python3.9/site-packages/integreat_cms/api/decorators.py", line 124, in wrap
  return function(request, *args, **kwargs)
File "/opt/integreat-cms/.venv/lib/python3.9/site-packages/integreat_cms/api/v3/locations.py", line 149, in locations
  result.append(transform_poi_translation(translation))
File "/opt/integreat-cms/.venv/lib/python3.9/site-packages/integreat_cms/api/v3/locations.py", line 75, in transform_poi_translation
  "title": poi.default_public_translation.title,
AttributeError: 'NoneType' object has no attribute 'title'

This is a regression from 70630ff (#2057).

@timobrembeck timobrembeck added 🐛 bug Something isn't working ‼️ prio: high Needs to be resolved ASAP. ☺️ effort: low Should be doable in <4h labels Feb 23, 2023
@timobrembeck timobrembeck added this to the 23Q1 milestone Feb 23, 2023
@seluianova seluianova self-assigned this Mar 13, 2023
@seluianova
Copy link
Contributor

seluianova commented Mar 14, 2023

It should not be possible to publish a POI translation in a different language when the default translation is not public

@timoludwig do you think it's okay if the translation is saved as a draft with a warning?
Like "The translation has not been published, because the default translation for this page is not public"

Or "Publish" button should be disabled/hidden at all?

Do we want to implement the same for Events and Pages for consistency?

@timobrembeck
Copy link
Member Author

@timoludwig do you think it's okay if the translation is saved as a draft with a warning? Like "The translation has not been published, because the default translation for this page is not public"

Or "Publish" button should be disabled/hidden at all?

Both is fine, probably a disabled publish button would be the most helpful (together with a small explanation text on hovering the button)?

Do we want to implement the same for Events and POIs for consistency?

I don't think so, for pages and events there might be content which is only relevant for a target group of a specific language, and thus region managers sometimes deliberately publish something only in one language. Just for locations this doesn't work because of the title...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
‼️ prio: high Needs to be resolved ASAP. 🐛 bug Something isn't working ☺️ effort: low Should be doable in <4h
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants