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

Wrap each addon settings form in their own form tag #10440

Merged

Conversation

MrPetovan
Copy link
Collaborator

@MrPetovan MrPetovan commented Jun 28, 2021

  • The single form tag was preventing a given addon settings to be saved if another addon had an empty required field.
  • Instead of concatenating the addon form HTML through Hook::callAll, we loop manually through the hooks, appending the HTML snippets to an array.

Depends on friendica/friendica-addons#1141, please merge the addon PR first.

Closes #10314
Closes #10439

- The single form tag was preventing a given addon settings to be saved if another addon had an empty required field.
- Instead of concatenating the addon form HTML through Hook::callAll, we loop manually through the hooks, appending the HTML to an array.
@MrPetovan MrPetovan force-pushed the bug/10439-addon-settings-forms branch from 847e50b to efff254 Compare June 28, 2021 03:15
@tobiasd tobiasd merged commit a44a9e7 into friendica:2021.06-rc Jun 29, 2021
@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

After merging this, I have no addon settings at all on my node. All themes are affected. Cleaning the SMARTY3 cache did not help.

In the PHP error log I see

PHP Notice:  Trying to access array offset on value of type null in /mod/settings.php on line 509

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

In the HTML source of the addon settings page I only see the wrapping HTML blocks

<form action="settings/addon" method="post" autocomplete="off">
	<input type="hidden" name="form_security_token" value="123456789">
	
</form>

@MrPetovan
Copy link
Collaborator Author

@tobiasd It looks like the select() on line 507 returns false when it really shouldn't. Can you test directly its return value?

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

The old SELECT * FROM `hook` WHERE `hook` = 'addon_settings'; is returning the expected results in the MySQL console I think. It lists 8 addons that look like the once I have enabled with user settings.

@MrPetovan
Copy link
Collaborator Author

It's odd that it's working on other people's node but not yours, so I'd be really curious to test the exact statement on your node.

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

So, what would be the exact statement be? Tell me and I'll throw it into the MariaDB console.

@MrPetovan
Copy link
Collaborator Author

I meant the PHP statement DI::dba()->select('hook', ['file', 'function'], ['hook' => 'addon_settings']), but the generated SQL statement should be SELECT `file`, `function` FROM `hook` WHERE `hook` = 'addon_settings'.

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

That yields to the same listing of addons.

@MrPetovan
Copy link
Collaborator Author

Is the blank display systematic? If so you might have to dig into the log for SQL errors.

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

It is systematic in that all users are affected yes. But the display is not blank - the Friendica UI around the listing is there. It is "just" the missing listing of the addons.

The MySQL error log is empty. I've increased the Friendica log level, but it looks like the log for the time when I visit the settings page does not hold any relevant information.

I have disabled all the addons that are returned by the query. From my understanding now there should be the info that no addons are configurable by the user right? This is not the case.

@MrPetovan
Copy link
Collaborator Author

Disabling the addons won't change anything, the PHP statement isn't returning an expected array so the foreach runs once with a null value no matter the contents of the hook table. I'm still puzzled as of why only your node (so far) is stumbling on this basic select statement.

Thanks for looking in the log, we still need to figure out the actual return value of the PHP statement.

@AlfredSK
Copy link

I'm still puzzled as of why only your node (so far)

I have checked it here. It looks like expected.

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

A print_r around the DBA->select yields

(
    [affected_rows] => 8
    [insert_id] => 0
    [num_rows] => 8
    [param_count] => 1
    [field_count] => 2
    [errno] => 0
    [error] => 
    [error_list] => Array
        (
        )

    [sqlstate] => 00000
    [id] => 108435
)```

And for those 8 rows the HTML form blocks are then inserted. But the `hook` information remains empty.

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

print_r(q("SELECT `file`, `function` FROM `hook` WHERE `hook` = 'addon_settings'"))

gives the expected array.

@MrPetovan
Copy link
Collaborator Author

Thank you, it looks like I should have used selectToArray() instead of select(), but I'm utterly puzzled about why it works for several nodes including mine and @AlfredSK 's but not yours.

@tobiasd
Copy link
Collaborator

tobiasd commented Jun 29, 2021

Indeed, with selectToArray I see the correct listing of the addon forms. And the settings seem to work (I can save changes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Side-Effects at Addon settings page Cat Avatar Generator usable for administrators only
3 participants