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

Report #8186

Merged
merged 10 commits into from
May 1, 2024
Merged

Report #8186

merged 10 commits into from
May 1, 2024

Conversation

live627
Copy link
Contributor

@live627 live627 commented Apr 26, 2024

  • The Report controller uses the new objects instead of raw SQL queries. Insert reference to Pinocchio saying "I'm a Real Boy now!"
  • Add a new report type for board access. Well actually it was moved from the board report in an attempt to be more accessible.
  • Use JavaScript to split long tables, increasing readability and eliminating the need to scroll vertically . Who wants to do that?

I also have some code that adds checkboxes to filter the results by category mainly useful for board permissions. Shall I add a commit for that?

@@ -335,11 +300,11 @@ public function boards(): void
$boardData['groups'] = implode(', ', $allowedGroups);

if (!empty(Config::$modSettings['deny_boards_access'])) {
$disallowedGroups = explode(',', $row['deny_member_groups']);
$disallowedGroups = explode(',', $board->deny_member_groups);
Copy link
Member

Choose a reason for hiding this comment

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

This line causes PHP to die with the following fatal error:

Fatal error: Uncaught TypeError: explode(): Argument #2 ($string) must be of type string, array given in /path/to/forum/Sources/Actions/Admin/Reports.php:303 Stack trace: #0 /path/to/forum/Sources/Actions/Admin/Reports.php(303): explode(',', Array) #1 [internal function]: SMF\Actions\Admin\Reports->boards() #2 /path/to/forum/Sources/Actions/Admin/Reports.php(194): call_user_func(Array) #3 /path/to/forum/Sources/Actions/Admin/Reports.php(642): SMF\Actions\Admin\Reports->execute() #4 [internal function]: SMF\Actions\Admin\Reports::call() #5 /path/to/forum/Sources/Actions/Admin/ACP.php(786): call_user_func(Array) #6 /path/to/forum/Sources/Actions/Admin/ACP.php(813): SMF\Actions\Admin\ACP->execute() #7 [internal function]: SMF\Actions\Admin\ACP::call() #8 /path/to/forum/Sources/Forum.php(263): call_user_func(Array) #9 /path/to/forum/index.php(146): SMF\Forum->execute() #10 {main} thrown in /path/to/forum/Sources/Actions/Admin/Reports.php on line 303

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment on lines 14 to 16

//~ positions.push(positions[positions.length - 1] + positions[positions.length - 2]);
//~ return;
Copy link
Member

Choose a reason for hiding this comment

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

Delete?

@Sesquipedalian
Copy link
Member

I also have some code that adds checkboxes to filter the results by category mainly useful for board permissions. Shall I add a commit for that?

Sounds nice. Yes, please!

@live627
Copy link
Contributor Author

live627 commented Apr 30, 2024

added those extra checkboxes

Copy link
Member

@Sesquipedalian Sesquipedalian left a comment

Choose a reason for hiding this comment

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

  1. The Board Access report uses the wrong title in the cat_bars.
  2. There is a useless duplicate table for each board in the Boards report.
    Screenshot 2024-04-30 at 2 49 17 PM Perhaps some more work needs to go into the logic that breaks up wide tables?
  3. The checkboxes don't do anything. I suggested changing <style> to <script> below, which is necessary. However, even after I did that in my own tests, the JavaScript ran into syntax errors that will need to be fixed.

Themes/default/Reports.template.php Show resolved Hide resolved
Themes/default/Reports.template.php Show resolved Hide resolved
@Sesquipedalian Sesquipedalian merged commit 429d519 into SimpleMachines:release-3.0 May 1, 2024
6 checks passed
@live627 live627 deleted the report branch May 1, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants