Fix pre-existing PII annotation debt and add pii_check to CI

未关闭
#845 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
4/5
预计耗时
3-5 天
新手友好度
68/100
Issue 类型
重构
描述清晰度
描述清楚
活跃度
活跃
技术栈
github-actions, python, yaml

调研方向

Start with .annotation_safe_list.yml, the model files under src/, and .github/workflows/ci.yml; run tox -e pii_check to reproduce the duplicate and uncovered-model failures. Review each listed model's data and add exactly one accurate annotation, remove the two redundant safelist entries, add pii_check to the CI matrix, and confirm the check passes with 100% coverage.

由索引模型根据 Issue 内容生成。

描述

Background

make pii_check runs code_annotations against every Django model in openedx-core's own
tree and requires each one to carry a .. no_pii: (or .. pii:) annotation, either inline in
the model's docstring or as an entry in .annotation_safe_list.yml. tox.ini's envlist
already includes a pii_check environment, but .github/workflows/ci.yml's test matrix lists
toxenv: ["django52", "package", "quality", "docs"], which does not include it. CI never runs
pii_check, so nothing currently blocks a merge that breaks it, and two problems have
accumulated on main as a result, confirmed by running tox -e pii_check directly:

Two models are double-covered, which is a lint error, not a coverage gap.
openedx_content.Draft and openedx_content.PublishableEntityVersion each carry an inline
.. no_pii: annotation in their own docstring (src/openedx_content/applets/publishing/models/draft_log.py
and .../models/publishable_entity.py), and each also has a now-redundant entry in
.annotation_safe_list.yml. code_annotations treats a model with both as annotated twice and
fails with a lint error rather than a coverage number:

openedx_content.Draft is annotated, but also in the safelist.
openedx_content.PublishableEntityVersion is annotated, but also in the safelist.

Twenty models are uncovered, confirmed by removing those two stale safelist entries and
re-running tox -e pii_check: 67 models total, 47 annotated, 20 uncovered, 70.1% coverage
against a 100% target. The twenty:

  • Seven oel_* backcompat shim models: oel_collections.Collection, oel_components.Component,
    oel_publishing.Container, oel_publishing.DraftChangeLog, oel_publishing.DraftChangeLogRecord,
    oel_publishing.LearningPackage, oel_publishing.PublishableEntity
  • Two openedx_catalog models: CatalogCourse, CourseRun
  • Three openedx_content models: ComponentVersionMedia, ContainerType, Media
  • Two models from the installed edx-organizations package: organizations.HistoricalOrganization,
    organizations.HistoricalOrganizationCourse
  • Six test-only models in test_django_app: ContainerContainer, ContainerContainerVersion,
    TestContainer, TestContainerVersion, TestEntity, TestEntityVersion

None of these twenty are models that #613 (the CBE competency models) adds, and none of #613's
own PRs touch them. This debt predates #613 and is unrelated to it.

What to do

  1. Remove the openedx_content.Draft and openedx_content.PublishableEntityVersion entries
    from .annotation_safe_list.yml; both are already covered by their own inline annotation.
  2. Annotate each of the twenty uncovered models above as .. no_pii: or .. pii:, whichever is
    accurate, either inline in the model's own docstring (for a model defined in this repo's
    src/) or in .annotation_safe_list.yml (for a model defined in an installed package, such
    as the two organizations.Historical* models).
  3. Add pii_check to the toxenv list in .github/workflows/ci.yml's test matrix, so a future
    PR that breaks PII coverage or introduces a double-covered model fails CI instead of only
    failing silently for whoever happens to run make pii_check locally.

Acceptance criteria

  • tox -e pii_check passes locally with no lint error and 100% coverage.
  • .github/workflows/ci.yml's test matrix includes pii_check in toxenv, and the CI run
    on the pull request shows a pii_check job that passes.
  • No model's annotation is both inline and in the safelist at once.
  • Every one of the twenty models listed above carries exactly one annotation, inline or in
    the safelist, accurately describing whether it stores personal data.

Out of scope

Anything in #613, #641, or #642: the CBE competency models, their own PII annotations, and the
openedx-platform safelist entries for them. This issue is unrelated to that work and does not
block or get blocked by it.

主要语言
Python
星标
10
派生
33
平均合并
4 天 1 小时
30 天内合并 PR
34

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

openedx/openedx-core 的其他 Issue

查看 openedx/openedx-core 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。