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 privacy and add reusable CI #82

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

keevan
Copy link
Contributor

@keevan keevan commented Sep 11, 2023

  • fix: privacy API issues for ccert_is table
  • chore: add reusable CI workflows
  • docs: add CI badge to README

Resolves #79

@keevan
Copy link
Contributor Author

keevan commented Sep 11, 2023

The CI badge would look like this on the README. Let me know if you want it tweaked / reordered.

image

@keevan
Copy link
Contributor Author

keevan commented Sep 11, 2023

Local tests before:

❯ ctrl test --filter 'provider_test::test_table_coverage'
Moodle 4.1.5+ (Build: 20230826), bababe5ab3dfac3528ab55728ca8ee9315a06931
Php: 8.0.29, pgsql: 14.0 (Debian 14.0-1.pgdg110+1), OS: Linux 5.19.0-50-generic x86_64
PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 00:00.848, Memory: 382.00 MB

There was 1 failure:

1) core_privacy\privacy\provider_test::test_table_coverage
The following tables with user fields must be covered with metadata providers:
  - local_recompletion_ccert_is (userid)

privacy/tests/privacy/provider_test.php:328
lib/phpunit/classes/advanced_testcase.php:80

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

After:

❯ ctrl test --filter 'provider_test::test_table_coverage'
Moodle 4.1.5+ (Build: 20230826), bababe5ab3dfac3528ab55728ca8ee9315a06931
Php: 8.0.29, pgsql: 14.0 (Debian 14.0-1.pgdg110+1), OS: Linux 5.19.0-50-generic x86_64
PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 00:00.858, Memory: 382.00 MB

OK (1 test, 0 assertions)

@keevan
Copy link
Contributor Author

keevan commented Sep 11, 2023

Interestingly, phpmd had been enabled previously with no seeming issue:

      - name: Run phpmd
        if: ${{ always() }}
        run: moodle-plugin-ci phpmd

There are a number of concerns raised which might require some deeper refactoring.

Would you prefer the CI for this PR turn it off for now, and separate issues raised for those phpmd violations?

@keevan keevan changed the title 79 fix privacy Fix privacy and add reusable CI Sep 11, 2023
- Had to enable phpmd check, as it is opt-in by default.
@keevan
Copy link
Contributor Author

keevan commented Sep 12, 2023

Turns out I misread the errors and phpcpd is the one returning an error.

Similar to how it has been running, I've disabled it so it doesn't make the CI red - with the idea is if it runs, it should be fixed and kept in a good state.

There are also some small modifications I made as I worked the file, in particular I moved the handling to a function and aligned the last bit of code for both bulkresetcompletion.php and editcompletion.php, so it's easier to compare and refactor at a later point in time if we want to tackle the phpcpd errors.

I can remove all those changes if you think it's easier to maintain in its previous state.

locallib.php Outdated Show resolved Hide resolved
- phpcpd issues remain, but now are matching 1:1
@danmarsden danmarsden merged commit af546f0 into danmarsden:MOODLE_311_STABLE Sep 13, 2023
10 checks passed
@keevan keevan deleted the 79-fix-privacy branch September 13, 2023 05:22
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.

Missing privacy api changes for custom cert changes.
2 participants