Admidio / admidio

Admidio is a free open source user management system for websites of organizations and groups. The system has a flexible role model so that it’s possible to reflect the structure and permissions of your organization.
https://www.admidio.org
GNU General Public License v2.0
304 stars 122 forks source link

Menu should be configurable and saved in database #339 #700

Closed sirnone closed 6 years ago

ximex commented 6 years ago

Sooo. I have done a quick overview. Things i found:

I will fix most of this after fixing this i will look again more in detail

@sirnone most of this things shouldn't happen with a good IDE. What IDE do you use?

Fasse commented 6 years ago

@ximex can we merge?

ximex commented 6 years ago

@Fasse no. I will push some fixes today and i hope the rest tomorrow

Fasse commented 6 years ago

Ok, no problem

ximex commented 6 years ago

There were many SELECT * added. Wouldn't it good if we change all SELECT * to SELECT column_name, ... to see exactly what columns we get and only return the columns we need?

Fasse commented 6 years ago

Yes, I would also prefer a SELECT column_name syntax.

ximex commented 6 years ago

more fixes are coming tomorrow. But please someone fix the $user bug

Fasse commented 6 years ago

The $user bug ?!?

ximex commented 6 years ago

Look comment above. $user is not defined

Fasse commented 6 years ago

Which comment? I cant See a comment with $user!

ximex commented 6 years ago

Here: https://github.com/Admidio/admidio/pull/700/files/c0088b1d9b262d86dd76e4d3c2e83c774233ba8f#diff-c1ab13b4ccae594442e8f0e2e2f70c18R182

ximex commented 6 years ago

The rest comes tomorrow. There are 4 files (htmlpage.php, tablecategory.php, tablemenu.php, menu_function.php) left where i have more to fix/improve.

@sirnone @Fasse could someone else does the SELECT * changes?

ximex commented 6 years ago

@sirnone @Fasse i think there are two foreign keys missing

ALTER TABLE %PREFIX%_menu
    ADD CONSTRAINT %PREFIX%_fk_men_men_parent  FOREIGN KEY (men_men_id_parent)  REFERENCES %PREFIX%_menu (men_id)                ON DELETE SET NULL ON UPDATE RESTRICT,
    ADD CONSTRAINT %PREFIX%_fk_men_com_id      FOREIGN KEY (men_com_id)         REFERENCES %PREFIX%_categories (cat_id)          ON DELETE RESTRICT ON UPDATE RESTRICT;
ximex commented 6 years ago

I'm happy now -> could be merged. Maybe i find some more but that could be done later in master too

Fasse commented 6 years ago

@ximex please test your changes:

Fatal error: Uncaught Error: Cannot use object of type stdClass as array in /admidio/adm_program/system/classes/htmlpage.php:750 Stack trace: #0 /admidio/adm_program/index.php(64): HtmlPage->showMainMenu() #1 {main} thrown in /admidio/adm_program/system/classes/htmlpage.php on line 750

ximex commented 6 years ago

@Fasse hmmm don't know were this changes get lost...

Fasse commented 6 years ago

it's done :)