pi-hole / web

Pi-hole Dashboard for stats and more
https://pi-hole.net
Other
2k stars 555 forks source link

Fix incorrect redirection #2993

Closed DL6ER closed 5 months ago

DL6ER commented 5 months ago

What does this implement/fix?

We should not try to apply settings levels (incl. all the consequences such as ill-fated redirects) when we have already detected that there are no expert-settings elements on this page.


Related issue or feature (if applicable): https://github.com/pi-hole/web/issues/2992

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:

rdwebdesign commented 5 months ago

Now Gravity Update page works, but All Settings page is empty (even using the toggle button).

DL6ER commented 5 months ago

Now we recall why the code was there :-) Should be fixed.

rdwebdesign commented 5 months ago

Now:

We need to run applyExpertSettings() in all pages, but we need a different redirect rule.

We should redirect only:

rdwebdesign commented 5 months ago

The original issue is $(".box:visible").length will be zero in 2 cases:

Suggestion:

Revert all commits and change this code:

    // If we left with an empty page (no visible boxes) after switching from
    // Expert to Basic settings, redirect to admin/settings/system instead
-   if ($(".box:visible").length === 0) {
+   if ($(".settings-selector").length !== 0 && $(".box:visible").length === 0) {
      window.location.href = "/admin/settings/system";
    }

This will trigger the redirect only if: