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

added homepaage locale setting #261

Merged
merged 6 commits into from
Feb 26, 2024
Merged

added homepaage locale setting #261

merged 6 commits into from
Feb 26, 2024

Conversation

Mudiwa66
Copy link
Contributor

Purpose

The importer was not picking up cases where the a a childpage was attempting to be linked to a homepage of a different locale. This caused the import to fail, without a descriptive reason to the user.

Approach

If a childpage attempts to link to a homepage that does not exist during import, the UI will return an error that will allow the user to fix it.

@Mudiwa66 Mudiwa66 requested a review from rudigiesler February 22, 2024 06:43
@@ -32,7 +32,8 @@
"wagtail_content_import.pickers.local",
"wagtail.admin",
"wagtail",
"wagtail.locales",
"wagtail_localize",
"wagtail_localize.locales",
Copy link
Contributor

Choose a reason for hiding this comment

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

According to https://wagtail-localize.org/latest/tutorial/2-configure-wagtail-localize/ , this should be placed between search and wagtail.contrib.forms.

It also says that wagtail_localize.locales is temporary and will be removed in future, so maybe we should stick with wagtail.locales?

requirements.txt Outdated
@@ -3,6 +3,7 @@ wagtail==5.0.5
wagtailmedia
psycopg2-binary
wagtail-content-import
wagtail-localize
Copy link
Contributor

Choose a reason for hiding this comment

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

Taking a brief look at the docs, this gives us integrations with external translation services, and the ability to work with PO files, which are not things that we've really needed for contentrepo. What does this give us that we want to include it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows the user to create a HomePage with a non default locale as far as I understand. Without it, the admin UI will still allow you to create locales, but the Homepage with that locale will not be created(which is something we want).

Copy link
Contributor

Choose a reason for hiding this comment

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

On my local instance (without these changes), if I go settings -> Locales, and then add a locale. I can then either go to the english homepage, click the kebab menu and select translate, which will copy that whole tree into the new locale, creating the new homepage with all the required pages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know that. That works for me too. I will remove it then.

@Mudiwa66
Copy link
Contributor Author

Test added :)

Copy link
Contributor

@rudigiesler rudigiesler left a comment

Choose a reason for hiding this comment

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

Just one small comment on the docs, otherwise it looks good :)

README.md Outdated
@@ -105,6 +105,9 @@ Authentication is required to access the API. Session, basic, and token authenti

To create an authentication token, you can do so via the Django Admin (available at the `/django-admin` endpoint), or by `POST`ing the username and password of the user you want to generate a token for to the `/api/v2/token/` endpoint.

### Internationalisation
To create or import pages in other languages, the user must first create the locale in the specified language. To create a new locale, go to "Settings" => "Locales" in the admin interface, and click "Add a new locale". Set the "Sync from" field to "English". This keeps the the specified language tree in sync with any existing and new English content as it's authored. After you press "Save", there should be two "Home" pages in the page explorer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this "Sync from" instruction from that additional library? I think we need to update this to where you select "Add translation" to the base language homepage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good observation. I fixed this.

@Mudiwa66 Mudiwa66 merged commit c5c2e28 into main Feb 26, 2024
4 checks passed
@Mudiwa66 Mudiwa66 deleted the fix-import branch February 26, 2024 07:50
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.

2 participants