Skip to content

823: Add example sentence to word list - #944

Open
JoeyStk wants to merge 5 commits into
mainfrom
add-example-sentence
Open

823: Add example sentence to word list#944
JoeyStk wants to merge 5 commits into
mainfrom
add-example-sentence

Conversation

@JoeyStk

@JoeyStk JoeyStk commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Short description

This PR adds the few pieces for #823. I decided to split the issue in multiple PRs, to keep it as reviewable as possible and more importantly, to ship the most important feature on the next occasion. From there we can slowly add the details. This also gives us the advantage of having short cycles for feedback.

Proposed changes

  • Show the example sentence as text (truncated as shown in the design, so that it doesn't visually clutter the view)
  • Show the example sentence audio (with a play button)
  • Show the Check Status for the example sentence and make it editable

How to test

  • Go to word list, see the column example sentence
  • Check if the status can be correctly changed
  • Check if the audio can be listened to
  • Check if the sentence shows up correctly (and is shortened, if the sentence is long)

Resolved issues

Fixes: Parts of #823

@JoeyStk
JoeyStk force-pushed the add-example-sentence branch from f83ae9d to 75bde9e Compare August 12, 2026 15:55
@deliverino

deliverino Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

LLM Review (verdigado-think)

1. Django Correctness

  • No model field changes were detected in the provided diff; therefore, no migrations are required.
  • New creation paths for Job/Unit/Word were not introduced (the PR only implements updates to existing instances), so attribution checks are not applicable here.

2. Internationalization

  • In lunes_cms/locale/de/LC_MESSAGES/django.po, the entry for msgid "more" is commented out (#~ msgid "more") and lacks a translation (msgstr). This string is used in WordAdminListRenderersMixin._generate_example_sentence_text_html (line 203). It should be properly translated.

3. Type Safety

  • New Python files are well-annotated using django-stubs types (e.g., _StrOrPromise).

4. Code Quality Gates

  • No obvious formatting drift or unjustified pylint disables were found. The # pylint: disable=too-many-arguments in WordAdminAssetWidgetsMixin._render_regenerate_widget is acceptable given the method's role as a generic HTML builder.

5. Security

  • CSRF Protection: The view update_word_example_sentence_check_status in lunes_cms/cmsv2/views/update_word_example_sentence_check_status.py is marked with @csrf_exempt. However, the corresponding TypeScript code in lunes_cms/src/example_sentence_check_status_update.ts (line 18) explicitly sends the X-CSRFToken header. There is no apparent reason to bypass CSRF protection on the server side if the client is providing the token; this should be removed to maintain security standards.

6. Testing

  • The PR introduces a new view (update_word_example_sentence_check_status) and significant administrative UI logic (renderers, widgets), but no corresponding tests were added under tests/. Tests should be provided for the new endpoint and the asset generation workflows.

8. Commit Message Style

  • The following commits do not follow the repository convention (<ticket-number-or-branch-slug>: <short imperative summary>):
    • 9c9fff1b: "Fix: Fix translation" (Missing ticket/slug, vague summary).
    • 8bcd9e18: "Fix: Fix prettier" (Missing ticket/slug).

@JoeyStk
JoeyStk marked this pull request as ready for review August 12, 2026 16:09
@JoeyStk
JoeyStk force-pushed the add-example-sentence branch from ae8bba6 to 9c9fff1 Compare August 13, 2026 09:52

@LeandraH LeandraH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great refactoring!

A couple of things:

Have you already gone through the points from the AI review? There might be some valid points in there.
Be careful when fixing the merge conflicts, you might accidentally overwrite #950
I think the audio of the sentence needs to be checked from the overview as well, so a play button there would be awesome!

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