Skip to content

Add ability to specify domains, lists, clients and group names as arrays#1629

Merged
DL6ER merged 17 commits into
development-v6from
tweak/domain_array
Oct 26, 2023
Merged

Add ability to specify domains, lists, clients and group names as arrays#1629
DL6ER merged 17 commits into
development-v6from
tweak/domain_array

Conversation

@DL6ER

@DL6ER DL6ER commented Sep 6, 2023

Copy link
Copy Markdown
Member

What does this implement/fix?

Add ability to specify domains, lists, clients and group names as arrays

Screenshot from 2023-09-06 18-47-17
Screenshot from 2023-09-06 18-47-23
Screenshot from 2023-09-06 18-47-27

Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

@DL6ER DL6ER mentioned this pull request Sep 6, 2023
Closed
@rdwebdesign rdwebdesign linked an issue Sep 6, 2023 that may be closed by this pull request
Regression: input multiple domains in domain input field adds them as a single domain #1621
Closed
@DL6ER DL6ER marked this pull request as ready for review September 7, 2023 17:44
@DL6ER

DL6ER commented Sep 7, 2023

Copy link
Copy Markdown
Member Author

Changing this to "ready for review" and kindly asking for the same

@DL6ER

DL6ER commented Sep 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

This branch continues to work with the current development-v6 web branch. However, if you specifically want to review the core of this PR (being able to add multiple at once), you'll need the web interface branch new/antigravity tweak/domain_array which contains the necessary changes for this PR (sending multiple domains as an array of strings).

edit See also the related PR pi-hole/web#2707.

This was referenced Sep 11, 2023
Merged
Merged
@DL6ER DL6ER requested a review from a team September 11, 2023 08:50
@DL6ER
Warn about spaces and newlines in domain/URLs when adding new items t…
c05e23e 
…o Pi-hole's lists

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Add ability to specify domains, lists, clients and group names as arr…
2c516d1 
…ays for creating multiple with the same properties (comment, groups, ...) at the same time

Signed-off-by: Dominik Derigs <dl6er@dl6er.de>
@DL6ER
Check eeach element for spaces
d4d7798 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Free row.items only after having used row.item one last time
eaaf398 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Ensure we free the row.items object only when we have actually alloca…
3552660 
…ted it (not if we are merely using a copy)

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Fix spellcheck error
1161463 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER force-pushed the tweak/domain_array branch from a755396 to 1161463 Compare September 15, 2023 15:36
@DL6ER

DL6ER commented Sep 15, 2023

Copy link
Copy Markdown
Member Author

Rebased on latest development-v6

@github-actions

github-actions Bot commented Sep 29, 2023

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@DL6ER
Merge branch 'development-v6' into tweak/domain_array
5e93277 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@github-actions

github-actions Bot commented Sep 29, 2023

Copy link
Copy Markdown

Conflicts have been resolved.

@DL6ER

DL6ER commented Oct 7, 2023

Copy link
Copy Markdown
Member Author

The spellcheck issue will be fixed in #1649 by 28358d4

DL6ER added 4 commits October 8, 2023 08:31
@DL6ER
Send processed object after list insertion to inform the client how m…
97cc855 
…any items have been successfully added, how many have failed and, if, what the individual errors were

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Also return a list of items that have been successfully added
425f791 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Add lists_processed to all corresponding OpenAPI elements
7e5357c 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Merge branch 'development-v6' into tweak/domain_array
23c082b 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@github-actions

github-actions Bot commented Oct 8, 2023

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions

github-actions Bot commented Oct 8, 2023

Copy link
Copy Markdown

Conflicts have been resolved.

@DL6ER
Include processed object only in POST and PUT responses
cd16053 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@pralor-bot

pralor-bot commented Oct 10, 2023

Copy link
Copy Markdown

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/adding-batch-adlists/65446/3

@DL6ER

DL6ER commented Oct 12, 2023

Copy link
Copy Markdown
Member Author

This PR has recently been affected by a bug solved in #1659 and should be reviewed thereafter.

@DL6ER

DL6ER commented Oct 13, 2023

Copy link
Copy Markdown
Member Author

This PR is now ready.

@yubiuser

yubiuser commented Oct 16, 2023

Copy link
Copy Markdown
Member

What's the reason to accept string and [string] and not just [string] for the relevant value? It could simply be an array of 1.

@DL6ER

DL6ER commented Oct 16, 2023

Copy link
Copy Markdown
Member Author

Following the REST principles, you should add/manipulate one entry per call so sending a string feels natural here. We simply extend this natural feeling by also allowing arrays. You are right, an array of size 1 is exactly identical to a naked string, however, to my understanding, rejecting plain strings and demanding them to be encapsulated in an otherwise empty array shell feels odd to me.

@DL6ER DL6ER enabled auto-merge October 18, 2023 17:05
yubiuser
yubiuser reviewed Oct 25, 2023
View reviewed changes
Comment thread src/api/docs/content/specs/common.yaml Outdated
Comment thread src/api/list.c
Comment thread src/api/list.c Outdated
DL6ER added 2 commits October 26, 2023 00:01
@DL6ER
Free regex after successful test compilation in API lists
9dbe107 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Use more precise examples in the API documentation
f2960cb 
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER requested a review from yubiuser October 26, 2023 15:07
@pralor-bot

pralor-bot commented Oct 26, 2023

Copy link
Copy Markdown

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/add-client-or-group-in-management-doesnt-work/65855/5

@DL6ER DL6ER merged commit c7f50a3 into development-v6 Oct 26, 2023
@DL6ER DL6ER deleted the tweak/domain_array branch October 26, 2023 20:34
@PromoFaux PromoFaux mentioned this pull request Feb 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yubiuser yubiuser yubiuser approved these changes

Assignees

No one assigned

Labels

Pi-hole v6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Regression: input multiple domains in domain input field adds them as a single domain

4 participants

@DL6ER @pralor-bot @yubiuser @rdwebdesign