thedigicraft / Atom.CMS

Atom.CMS
56 stars 50 forks source link

Settings table - condense the amount of code #181

Open dbashby opened 8 years ago

dbashby commented 8 years ago

I currently have all of the settings table outputting the data, site-title, debug and admin title. It is all working through the use of classes however for the small amount of data I am retrieving I have a lot of code and I am trying to condense it into 1 prepared statement.

In settings.php I currently have

`require 'classes.php';

Calls the settings class

//debug panel 
$id = "debug-status";
$stmt = $pdo->prepare("SELECT \* FROM settings WHERE id = :id");
$stmt->bindParam(":id",$id,PDO::PARAM_STR);
if($stmt->execute()){
    $rows = $stmt->fetchObject("Debug");
    $debug = $rows->get('value');
}`

`//Site title $id = "site-title"; $stmt = $pdo->prepare("SELECT * FROM settings WHERE id = :id limit 1"); $stmt->bindParam(":id",$id,PDO::PARAM_STR); if($stmt->execute()){

    $rows = $stmt->fetchObject("Site_title");
    $siteTitle = $rows->get('value');
}`

`//Admin title $id = "admin-title"; $stmt = $pdo->prepare("SELECT * FROM settings WHERE id = :id limit 1"); $stmt->bindParam(":id",$id,PDO::PARAM_STR); if($stmt->execute()){

    $rows = $stmt->fetchObject("Admin_title");
    $adminTitle = $rows->get('value');
}`

I realise that as I am fetching an object I will only get 1 result but when I try to fetchAssoc I get nothing in return

In my classes.php I also have 3 sets of code each 1 just getting the 1 value

#Debug Panel: class Debug { public function get($debug) { if(property_exists($this, $debug)) { return $this->$debug; } return false; } }

And bar the variables the other 2 pieces of code are the same returning the value for the debug panel, the site title and the admin site title.

There must be a way to condense this code into 1 far simpler prepared statement but I cannot find it. I have had the array outputting the whole table with

`$stmt = $pdo->prepare("SELECT * FROM settings"); $stmt->bindParam(1,$id,PDO::PARAM_STR); $stmt->execute(); echo "

";
while($row = $stmt->fetch(PDO::FETCH_ASSOC)){

    echo 'Echo Output: ' . 'id: ' . $row['id'] . ' ' . 'label: ' . $row['label'] . ' ' . 'value: ' . $row['value'] .'<br>';
    //echo var_dump($row);

}
echo "</pre>";

`

But I cannot seem to work out why I cannot retrieve all the data and then echo each element out.

settingstable

creptor commented 8 years ago

I haven't look up if this is a correct way to handle this, but it will make your site faster, and your code less populated.

If you don't understand the code I have basically created a PDO call that makes the id a variable, storing it's value in the process.

Code:

function create_options(){
    $conn=$pdo->query('SELECT id,value FROM settings');
    if($conn->rowCount()>0){
        foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){
            global $$data['id'];
            $$data['id']=$data['value'];
        }
    }
}
create_options();

Note: fetch object is intended to work with clases (class) in PHP, not in the way you're treating them.

dbashby commented 8 years ago

Unfortunately once I put this function in place of my previous code I get nothing returned when I try to reload the page. I have looked through your code and can only see the $pdo not defined in the query so am I right in saying that that is all that needs to be in the brackets?

function create_options($pdo){ }

As well as create_options($pdo);

Thanks

creptor commented 8 years ago

You're right, sorry, it was my mistake. The code was taken from somewhere where I didn't need to define the $pdo. Making that change should fix it.

dbashby commented 8 years ago

Unfortunately not. Nothing is returned I will take another look at it in the morning.

creptor commented 8 years ago

the idea of the script is that it doesn't return anything (no retun by the way), it just sets up the options variables. To test it, you must comment out the real variables, or no noticiable changes will be visible.

dbashby commented 8 years ago

Sorry it's been a while. So I just place the function inside the debug panel under the relevant listing and all the information should be displayed?

create_options($pdo);

So instead of

<?php echo '<pre>'; ?> <?php echo 'Database Debug Value Set: ' . $debug . '<br>'; ?> <?php echo 'Site Title: ' . $siteTitle . '<br>'; ?> <?php echo 'Admin Title: ' . $adminTitle . '<br>'; ?> <?php echo '</pre>'; ?>

I should just have

create_options($pdo);

Is this correct?

The reason I ask is that after adding this function and loading up index.php I get a blank page.

creptor commented 8 years ago

no, the create_options function that I gave you only sets the variable and values, it does nothing else, you would still need to call them where you need it.

Example:

//in database => ('id' => 'title' , 'value' => 'My site')
create_options($pdo);
//---->this returns $title='My site';

echo $title
//displays the $title variable
dbashby commented 8 years ago

I am sorry but I must be missing something, when I try to use the function I get a blank page returned. My understanding from the above is that say on the index page.

echo $pageTitle | $siteTitle is no good now I need ? As I say I have tried all sorts, even putting the function, create_options($pdo); between the title tags but I do not get anything.
dbashby commented 8 years ago

Just looking at the code again, do I need to declare $data in the function brackets as well?

function create_options($pdo, $data){ $conn=$pdo->query('SELECT id,value FROM settings'); if($conn->rowCount()>0){ foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){ global $$data['id']; $$data['id']=$data['value']; } } }

i.e. create_options($pdo, $data);

Although I have tried this and still only blank page returned.

creptor commented 8 years ago

no, $data is not necessary, please add a error_reporting(-1); at the start of your database connection, so every connection error (with php) is displayed.

dbashby commented 8 years ago

The error returned

Parse error: syntax error, unexpected '[', expecting ',' or ';' in >C:\xampp\htdocs\1mysite\php_inc\setup.php on line 46

Line 46 is

global $$data['id'];

of the function

function create_options($pdo, $data){ $conn=$pdo->query('SELECT id,value FROM settings'); if($conn->rowCount()>0){ foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){ global $$data['id']; $$data['id']=$data['value']; } } }

creptor commented 8 years ago

please use pastebin to upload your setup.php

dbashby commented 8 years ago

I have put in new file due to volume of code

http://pastebin.com/6jZFaArU

As a side question is using file_exists(); the best way to go about covering any 404 errors ? Thanks

creptor commented 8 years ago

no, 404 error is synonymous of not found, and file_exists() check if the file does exists, which is not the same. also they use different check functions, a 404 error is for html headers, and not system files (it can still work dough).

Also be cautious, because your PHP server may cache the result of this function, resulting in misleading results.

This also has some noticeable problems like this one:

Warning This function returns FALSE for files inaccessible due to safe mode restrictions. However these files still can be included if they are located in safe_mode_include_dir.

Quoted from php.net

For checking URIs response is better to get the response headers, use a code like this one (there're many ways to do this).

$file = 'http://www.domain.com/somefile.jpg';
$file_headers = @get_headers($file);
if($file_headers[0] == 'HTTP/1.1 404 Not Found') {
    $exists = false;
}
else {
    $exists = true;
}
creptor commented 8 years ago

I have found out why my code didn't work, it was the variables....

Example:

//variable name needed `$siteTitle` (for the title)

**->execution of my function

//variable gotten: `$site-title`

Note that $site-title is different than $siteTitle, which is why you're getting some blank stuff. To fix this, you have to change (in the database) the id column, to match the needed variable name.

That's what basically my code does, makes the id string a variable, and it inserts the value to it :3

creptor commented 8 years ago

Also my code has no error... I have it working, and that error makes no sense.

Try checking your included files, this page works like a charm http://phpcodechecker.com/

dbashby commented 8 years ago

settingsdb

To fix this, you have to change (in the database) the id column, to match the needed variable >name.

As you see the column name in the database 'id' for the title is site-title, so it is the variables in the pages I need to correct to fit yes?

I do appreciate the time your spending on this, so thanks

So should I change the id in the DB from site-title to siteTitle and then try your function?

dbashby commented 8 years ago

I have changed the appropriate variables to match with the DB

settingsdb

but still getting the

Parse error: syntax error, unexpected '[', expecting ',' or ';' in >C:\xampp\htdocs\1mysite\php_inc\setup.php on line 46

dbashby commented 8 years ago

UPDATE: Just realised that prior to using error_reporting(-1) I was using (0) so I wouldn't have seen them. The only 2 pages being called in are

include 'connection/connect.php'; require 'functions.php';

We know the connection is working along with functions.php.

I have checked those pages again and nothing is flagged as being an issue.

Update:

I can't see how this is the issue but it has just popped up

Undefined variable: debug in C:\xampp\htdocs\1mysite\template\navigation.php on line 2

Notice: Undefined variable: debug in C:\xampp\htdocs\1mysite\index.php on line 84

I have no idea where they have come from

dbashby commented 8 years ago

I have found 1 error message, I had to force the error though. I just added random letters to the url so

http://localhost/1mysite/index.php?page=home

became

http://localhost/1mysite/index.php?page=homefggd

The error I got was

Warning: Invalid argument supplied for foreach() in C:\xampp\htdocs\1mysite\php_inc\setup.php on >line 68

That is the foreach loop in the function

foreach($page as $row){

$page = data_page($pdo, $pageid); foreach($page as $row){ $data=$row; //var_dump($data); } return $data;

Could this be why the function is not working even though they are not connected as 1 is dealing with the settings table and the other is working with the posts table?

I also loose the page information in the tab, obviously thats to be expected as the page

http://localhost/1mysite/index.php?page=homefggd does not exist

At the same time I also got

Undefined variable: data in C:\xampp\htdocs\1mysite\php_inc\setup.php on line 72

But again I cannot see how that would stop the function below from working?

function create_options(){ $conn=$pdo->query('SELECT id,value FROM settings'); if($conn->rowCount()>0){ foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){ global $$data['id']; $$data['id']=$data['value']; } } } create_options();

dbashby commented 8 years ago

I closed the browser down and restarted it, unfortunately all I get now for the site is

Parse error: syntax error, unexpected '[', expecting ',' or ';' in >C:\xampp\htdocs\1mysite\php_inc\functions.php on line 27

and a white page

Update:

The site is functioning now however the siteTitle is missing again and the debug button has vanished. The value is 1 and the button was there before. No idea whats going on as it was working prior to the browser being closed.

<?php echo $page["title"] . ' | ' . $siteTitle; ?>

settingsdb

creptor commented 8 years ago

Alright, I'll check if it's necessary to add something else to my function to work properly, and which php version is needed to run it.

dbashby commented 8 years ago

Just so you know, I am running 7.0.3

I am just going through making sure all the variables I used match the DB table,

All variables are set and match the database table.

creptor commented 8 years ago

I'm sorry I think I have lost it... please upload again an image of how your site looks, and the functions file (please separate them, that way the line numbers match).

dbashby commented 8 years ago

Ok, no problem. Here are all the pages in seperate files. http://pastebin.com/29WNw1jR - Index http://pastebin.com/gE9UvqAe - Functions http://pastebin.com/SkkrD9Du - setup http://pastebin.com/PL6fiMjc - Config - not much in this http://pastebin.com/C6FzdVFp - Connection

I think that is everything, let me know if you need anything else. Thanks

poststable

creptor commented 8 years ago

I didn't explain myself properly. With the image I was referring to how the site look, that way I can find errors easier. Or at least know the file where they are.

Please upload a image of how your browser process your site.

dbashby commented 8 years ago

Here are the screenshots of the basic site depending on desktop or mobile device. Hopefully this helps?

mobileview

desktopview

creptor commented 8 years ago

The images show no site errors, do you still ave some of them?, please if so copy the hole error output.

dbashby commented 8 years ago

I commented out the function you wrote for me and am just running the classes at the moment until the issue is found. The errors come up due to the function that gets all the settings from the settings table. The error I get is

Parse error: syntax error, unexpected '[', expecting ',' or ';' in >C:\xampp\htdocs\1mysite\php_inc\functions.php on line 66

This relates to the

#Settings table function function create_options($pdo, $data){ $conn=$pdo->query('SELECT id,value FROM settings'); if($conn->rowCount()>0){ foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){ global $$data['id']; $$data['id']=$data['value']; } } } create_options($pdo);

Line 66 is

global $$data['id'];

I do not get any of the site, just a white screen.

Not all the time but every now and again I am also getting the following error

UPDATE

The foreach was left from when I was testing things out, it is not needed so have removed and the erorr for the foreach only has gone. I am still getting the above error with the function above.

Warning: Invalid argument supplied for foreach() in C:\xampp\htdocs\7cms\config\setup.php on line >23

This relates to

#Gets data for all pages - Turned into a function $page = data_page($pdo, $pageid); foreach($page as $row){ $data=$row; } return $data;

Specifically

foreach($page as $row){

creptor commented 8 years ago

try replacing my function with this:

function create_options($pdo){
    $conn=$pdo->query('SELECT id,value FROM settings');
    if($conn->rowCount()>0){
        foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){
            $key=$data['id']
            global $$key;
            $$key=$data['value'];
        }
    }
}
create_options($pdo);

Also, try to comment out the foreach($page as $row){ $data=$row; } return $data;, to see if something gets broken. If is that the case please tell me what is it, if not just leave it that way :+1:

dbashby commented 8 years ago

I replaced the old function with the new, I got an error

Parse error: syntax error, unexpected 'global' (T_GLOBAL) in >C:\xampp\htdocs\1mysite\php_inc\functions.php on line 67

error1

creptor commented 8 years ago

.-. , maybe is the Apache configuration, here, this code should work, but is more extensive than the last and could result in unexpected behavior.

function create_options($pdo){
    $conn=$pdo->query('SELECT id,value FROM settings');
    if($conn->rowCount()>0){
        $return=array();
        foreach($conn->fetchAll(PDO::FETCH_ASSOC) as $data){
            $return[]=array($data['id'],$data['value']);
        }
        return $return;
    }
}
//returns array(x){'0'=>array(2)('the_id','the_value'), ..... }
foreach(create_options($pdo) as $data=>$value){
    $$value[0]=$value[1];
}
//returns variables like: $siteTitle='The option value';
dbashby commented 8 years ago

Unfortunately not, I get the following and as you see I lose the siteTitle, I realise that due to the error I would but just pointing it out.

arraytostring

arraytostring2

dbashby commented 8 years ago

Has anyone come up with a fix for this? Thanks

creptor commented 8 years ago

maybe try:

$$data[$value][0]=$data[$value][1];