Add private checkbox to check repositories#134
Conversation
Add 'Do not share publicly' checkbox per repository entry in the CODECHECK metadata form. Private repositories are stored with an isPrivate flag and filtered from the generated codecheck.yml. Backward compatible with existing comma-separated repository format.
|
@subhanLabs please check exposure of data to the landing page (and other exports), then remove the "draft" status again. |
nuest
left a comment
nuest
left a comment
There was a problem hiding this comment.
- Please add a couple of screenshots how this looks in the UI.
- Please add multiple test cases that ensures correct access to the information in any form or API endpoint or export.
This is a seemingly small feature, but it is one that can really break the trust in the tool for authors and editors if it is not perfect.
| ? parsed | ||
| : data.codecheck.repository.split(',').map(r => ({ url: r.trim(), isPrivate: false })).filter(r => r.url); | ||
| } catch { | ||
| // Backward compat: old plain comma-separated string |
There was a problem hiding this comment.
For what is this backward compatibility important? We do not have a release of the software yet, so no existing installations do have the "old" data stored.
There was a problem hiding this comment.
Good point, removed the backward compat code in the refactor commit.
|
@subhanLabs Same here - let's resolve the merge conflicts before the review. This also looks like you have some boilerplae code going for the addtional check box about the source of the codecheck.yml, which is good! |
Screenshots added above showing the checkbox in the UI with the tooltip visible on hover. For test cases: added three PHPUnit unit tests for the Regarding API/export exposure, checked |
Private repositories were being exposed on the article landing page
sidebar because getRepository() was returning the raw JSON string.
Now parses the new {repositories, repoWithCodecheckYaml} format and
returns only public URLs as a comma-separated string.
|
|
||
| $decoded = json_decode($raw, true); | ||
|
|
||
| if (is_array($decoded) && isset($decoded['repositories']) && is_array($decoded['repositories'])) { |
There was a problem hiding this comment.
What if the structure here is not as expected?
At least add an else clause with an error/warning log that something is not working as expected.
| fn($r) => isset($r->url) ? $r->url : '', | ||
| array_filter($repository->repositories, fn($r) => empty($r->isPrivate)) | ||
| )); | ||
| $publicUrls = array_filter($publicUrls); |
There was a problem hiding this comment.
Add a debug log statement here to the effect of "filtered out x of n repositories because they are marked as private".
| public function getRepository(): string | ||
| { | ||
| return $this->data['repository'] ?? ''; | ||
| $raw = $this->data['repository'] ?? ''; |
There was a problem hiding this comment.
Suggest to rename item to repositories for clarity, and to not return an empty string but to return an empty array as the "no values" option.
|
|
||
| cy.get('.repo-private-checkbox').eq(0).should('be.checked'); | ||
| cy.get('.repo-private-checkbox').eq(1).should('not.be.checked'); | ||
| }); |
There was a problem hiding this comment.
Add one more test here: click save, and then the state should be properly preserved. Ideally do not match bit index number, but by the value entered in the field.
2 participants
closes #100
Add
Do not share publiclycheckbox per repository entry in the CODECHECK metadata form. Private repositories are stored with anisPrivateflag and filtered from the generatedcodecheck.yml. Backward compatible with existing comma-separated repository format.Repository excluded from the yml preview & download:
Private excluded from the article landing page:
