MyBB Community Forums

In admin/modules/config/profile_fields.php, I believe the admin_config_profile_fields_add_commit hook should be moved above the insert query for the add function. If you look at the edit function, the hook comes right before the update, allowing you to modify the array before being inserted.

Current:

$fid = $db->insert_query("profilefields", $new_profile_field);

$db->write_query("ALTER TABLE ".TABLE_PREFIX."userfields ADD fid{$fid} TEXT");

$plugins->run_hooks("admin_config_profile_fields_add_commit");

Suggested:

$plugins->run_hooks("admin_config_profile_fields_add_commit");

$fid = $db->insert_query("profilefields", $new_profile_field);

$db->write_query("ALTER TABLE ".TABLE_PREFIX."userfields ADD fid{$fid} TEXT");

Unless this was intentional? As is it does return the fid, so I guess you could write a separate query off of that? I just find it odd that both functions (add and edit) don't agree on order.

(2015-06-21, 07:55 PM)spork985 Wrote: [ -> ]I just find it odd that both functions (add and edit) don't agree on order.

Inconsistency - nothing unusual in MyBB 1.x.

I agree it doesn't make much sense and it'd be nice to correct it, but there are probably more issues like that (even though some of them were fixed in 1.8.x) and they're rather a very low priority + changing the order may corrupt current plugins.

1.8 is still rather "new". I think it should be corrected before more plugins are developed. I'm not sure how it can be "low priority". All that needs done is 2 lines switched.

(2015-06-22, 12:04 AM)spork985 Wrote: [ -> ]1.8 is still rather "new".

Nearly 2 months to the 1st release anniversary - I wouldn't call it new even in quotes.

(2015-06-22, 12:04 AM)spork985 Wrote: [ -> ]I think it should be corrected before more plugins are developed.

Here I can't disagree. However:

(2015-06-22, 12:04 AM)spork985 Wrote: [ -> ]I'm not sure how it can be "low priority".

And I'm not sure how can it be a higher priority compared to lots of other bugs. As you said, you can use a workaround for this. It's not optimal, another query is needed, but breaking stuff to avoid it is not a solution.

(2015-06-22, 12:04 AM)spork985 Wrote: [ -> ]All that needs done is 2 lines switched.

2 line changes which would stop all plugins that globalise $fid from working due to previous implementation. That does matter. And it matters very much because the fid of new profile fields wouldn't be accessible from any hook at all if we rushed and implemented your suggested change - and this should answer one of your questions - this inconsistency is sort of intentional due to unknown fid before the insert query. It can be possibly fixed different ways (for example by adding another hook, but then I doubt we want 2 hooks next to eachother), but switching 2 lines wouldn't definitely be sufficient to cover everything that needs to be covered.

Alright, fair enough. This can be closed.

Rejecting.