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.

Sign up for GitHub

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

Jump to bottom

Show all available data on Long-term data query log page #2202

Merged
merged 9 commits into from May 31, 2022
Merged

Show all available data on Long-term data query log page #2202

merged 9 commits into from May 31, 2022

Conversation

rdwebdesign
Copy link
Member

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)
  • I have Signed Off all commits. (git commit --signoff)

What does this PR aim to accomplish?:

Fix #2149.

How does this PR accomplish the above?:

Add missing columns reply_type, reply_time, dnssec to the query log table.

What documentation changes (if any) are needed to support this PR?:

none

@rdwebdesign
Add missing columns reply_type, reply_time, dnssec
7fb8f94 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign requested review from yubiuser and a team May 15, 2022 22:18
@rdwebdesign
Fix multiplier (reply_time field unit is second)
ba18ece 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Fix prettier
9d225ed 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign linked an issue May 15, 2022 that may be closed by this pull request
Long-term data query log page does not show all available data of the database #2149
Closed
@rdwebdesign
Replace queries with query_storage
98a0e82 
- use client names, when available;
- use column names instead of indexes;

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
yubiuser
yubiuser reviewed May 29, 2022
View reviewed changes
api_db.php Outdated Show resolved Hide resolved
@rdwebdesign
Use IP if there is no client name
da85d1b 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Fix DNSSEC info
2dd5fb8 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Fixing prettier line break
fad13b1 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
yubiuser
yubiuser reviewed May 30, 2022
View reviewed changes
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Approving, only small nitpicks to match what we have on the query log. You could also change it there if you want.

Additionally, in long-term we map the type to

https://github.com/pi-hole/AdminLTE/blob/6c320a42574f9f0944ded71d84b7331557dbc6ea/scripts/pi-hole/php/func.php#L478

And set A (IPv4), and AAA (IPv6). In the query log the type comes via API as A and AAAA (without the IPvX). I'm not sure which I like more. If we don't adjust it here, I can PR a change to FTL.

What do you think?

scripts/pi-hole/js/db_queries.js Show resolved Hide resolved
scripts/pi-hole/js/db_queries.js Show resolved Hide resolved
scripts/pi-hole/js/db_queries.js Show resolved Hide resolved
@rdwebdesign
Use the same types, as returned by FTL
cd41520 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@pralor-bot
Copy link

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

https://discourse.pi-hole.net/t/resolve-client-ip-adresses-for-the-long-term-data-section/55655/2

@rdwebdesign
Use consistent output for both query lists
33c5450 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@yubiuser yubiuser merged commit ffaad6d into devel May 31, 2022
@yubiuser yubiuser deleted the fix/issue2149 branch May 31, 2022 19:07
@pralor-bot
Copy link

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

https://discourse.pi-hole.net/t/add-hostname-of-clients-to-long-term-query-database/50103/11

@pralor-bot
Copy link

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

https://discourse.pi-hole.net/t/top-clients-long-term-data/56325/2

@yubiuser yubiuser mentioned this pull request Jul 4, 2022
Merged
1 task
@pralor-bot
Copy link

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

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-16-web-v5-13-and-core-v5-11-1-released/56384/1

@yubiuser yubiuser mentioned this pull request Jul 16, 2022
Merged
1 task
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Long-term data query log page does not show all available data of the database
3 participants
@rdwebdesign @pralor-bot @yubiuser