Merge pull request #626 from panique/develop

Develop
panique · Mar 7, 2015 · f978a88 · f978a88
2 parents 7df7612 + ec7f5c7
commit f978a88
Show file tree

Hide file tree

Showing 30 changed files with 681 additions and 302 deletions.
diff --git a/.scrutinizer.yml b/.scrutinizer.yml
@@ -0,0 +1,5 @@
+# This file just tells the wonderful code quality analyzer Scrutinizer (https://scrutinizer-ci.com/g/panique/huge/)
+# that we are using external services (Travis) to generate code coverage stats
+# TODO is this correct ?
+tools:
+    external_code_coverage: true
diff --git a/.travis.yml b/.travis.yml
@@ -0,0 +1,30 @@
+language: php
+
+php:
+    - 5.5
+    - 5.6
+    - hhvm
+
+before_install:
+- sudo apt-get update > /dev/null
+
+before_script:
+   - sudo apt-get install apache2
+   - sudo a2enmod rewrite
+   # configure apache virtual hosts, create vhost via travis-ci-apache file template
+   - sudo cp -f travis-ci-apache /etc/apache2/sites-available/default
+   - sudo sed -e "s?%TRAVIS_BUILD_DIR%?$(pwd)?g" --in-place /etc/apache2/sites-available/default
+   - sudo service apache2 restart
+   # composer
+   - composer self-update
+   - composer install --prefer-source --no-interaction --dev
+   # go to tests folder
+   - cd tests
+
+# run unit tests, create result file
+script: phpunit --configuration phpunit.xml --coverage-text --coverage-clover=coverage.clover
+
+# gets tools from Scrutinizer, uploads unit tests results to Scrutinizer (?)
+after_script:
+  - wget https://scrutinizer-ci.com/ocular.phar
+  - php ocular.phar code-coverage:upload --format=php-clover coverage.clover
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,10 +1,29 @@
 # CHANGE LOG
 
+For the newest (und unstable) version always check the develop branch.
+
+## 3.1
+
+Code Quality at Scrutinizer 9.7/10, at Code Climate 3.9/4
+
+**February 2015**
+
+- [panique] several code quality improvements (and line reductions :) ) all over the project
+- [PR](https://github.com/panique/huge/pull/620) [owenr88] view rending now possible with multiple view files
+- [panique] lots of code refactorings and simplifications all over the project
+- [PR](https://github.com/panique/huge/pull/615) [Dominic28] Avatar can now be deleted by the user
+- [panique] First Unit tests :)
+- [panique] several code quality improvements all over the project
+- [panique] avatarModel code improvements
+- [panique] renamed AccountType stuff to UserRole, minor changes 
+
 ## 3.0
 
+Code Quality at Scrutinizer 9.3/10, at Code Climate 3.9/4
+
 **February 2015**
 
-- [panique] AccountTypeModel reduced to one method (removed duplicate code)
+- [panique] removed duplicate code in AccountTypeModel
 - [PR](https://github.com/panique/huge/pull/587) [upperwood] Facebook stuff completely removed from SQL
 - [panique] tiny text changes
 

diff --git a/README.md b/README.md
@@ -2,6 +2,11 @@
 
 # HUGE
 
+[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/panique/huge/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/panique/huge/?branch=master)
+[![Code Climate](https://codeclimate.com/github/panique/huge/badges/gpa.svg)](https://codeclimate.com/github/panique/huge)
+[![Travis CI](https://travis-ci.org/panique/huge.svg?branch=master)](https://travis-ci.org/panique/huge)
+[![Dependency Status](https://www.versioneye.com/user/projects/54ca11fbde7924f81a000010/badge.svg?style=flat)](https://www.versioneye.com/user/projects/54ca11fbde7924f81a000010)
+
 Just a simple user authentication solution inside a super-simple framework skeleton that works out-of-the-box
 (and comes with an auto-installer), using the future-proof official bcrypt password hashing/salting implementation of 
 PHP 5.5+, plus some nice features that will speed up the time from idea to first usable prototype application 
@@ -16,11 +21,7 @@ applications that - surprisingly and intentionally - go back to the basics of pr
 static classes, extremely simple constructs, not-totally-DRY code etc. while keeping the code extremely readable 
 ([StackOverflow](http://www.dev-metal.com/architecture-stackoverflow/), Wikipedia, SoundCloud).
 
-Buzzwords: [KISS](http://en.wikipedia.org/wiki/KISS_principle), [YASNI](http://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it). 
-
-[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/panique/huge/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/panique/huge/?branch=master)
-[![Code Climate](https://codeclimate.com/github/panique/huge/badges/gpa.svg)](https://codeclimate.com/github/panique/huge)
-[![Dependency Status](https://www.versioneye.com/user/projects/54ca11fbde7924f81a000010/badge.svg?style=flat)](https://www.versioneye.com/user/projects/54ca11fbde7924f81a000010)
+Buzzwords: [KISS](http://en.wikipedia.org/wiki/KISS_principle), [YASNI](http://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it).
 
 #### Quick-Index 
 
@@ -256,6 +257,23 @@ Then check your server's IP / domain. Everything should work fine.
 
 By default HUGE has a demo-user: username is `demo`, password is `12345678`. The user is already activated.
 
+### What the hell are .travis.yml, .scrutinizer.yml etc. ?
+
+There are several files in the root folder of the project that might be irritating:
+
+ - *.htaccess* (optionally) routes all traffic to /public/index.php! If you installed this project correctly, then this
+   file is not necessary, but as lots of people have problems setting up the vhost correctly, .htaccess it still there
+   to increase security, even on partly-broken-installations.
+ - *.scrutinizer.yml* (can be deleted): Configs for the external code quality analyzer Scrutinizer, just used here on
+   GitHub, you don't need this for your project.
+ - *.travis.yml* (can be deleted): Same like above. Travis is an external service that creates installations of this
+   repo after each code change to make sure everything runs fine. Also runs the unit tests. You don't need this inside
+   your project.
+ - *composer.json* (important): You should know what this does. ;) This file says what external dependencies are used.  
+ - *travis-ci-apache* (can be deleted): Config file for Travis, see above, so Travis knows how to setup the Apache.    
+
+*README* and *CHANGELOG* are self-explaining.
+
 #### Documentation <a name="documentation"></a>
 
 A real documentation is in the making. Until then, please have a look at the code and use your IDE's code completion 
@@ -324,3 +342,12 @@ https://github.com/panique/huge/issues?state=open
 - [Notes on password & hashing salting in upcoming PHP versions (PHP 5.5.x & 5.6 etc.)](https://github.com/panique/huge/wiki/Notes-on-password-&-hashing-salting-in-upcoming-PHP-versions-%28PHP-5.5.x-&-5.6-etc.%29)
 - [Some basic "benchmarks" of all PHP hash/salt algorithms](https://github.com/panique/huge/wiki/Which-hashing-&-salting-algorithm-should-be-used-%3F)
 - [How to prevent PHP sessions being shared between different apache vhosts / different applications](http://www.dev-metal.com/prevent-php-sessions-shared-different-apache-vhosts-different-applications/)
+
+### Side-facts
+
+1. Weird! When I renamed php-login to HUGE (to get rid off the too generic project name and to make it fitting nicely
+   to MINI, TINY and MINI2, my other projects) I had a research if the word "huge" is already used in the php world for 
+   sure. Nothing came up. Then, weeks later, I stumbled upon this: https://github.com/ffremont/HugeRest
+   I nice little framework in PHP, but it has only 1 star on Github, so it's obviously not so widely used. Looks very 
+   professional, too. Hmm.... The guy behind published the entire readme etc. in pure french (!), so it's hard to use 
+   for non-french-speaking people. However, I'm not related to him in any way, this is pure coincidence.
diff --git a/application/config/texts.php b/application/config/texts.php
@@ -23,15 +23,14 @@
 	"FEEDBACK_USER_EMAIL_ALREADY_TAKEN" => "Sorry, that email is already in use. Please choose another one.",
 	"FEEDBACK_USERNAME_CHANGE_SUCCESSFUL" => "Your username has been changed successfully.",
 	"FEEDBACK_USERNAME_AND_PASSWORD_FIELD_EMPTY" => "Username and password fields were empty.",
-	"FEEDBACK_USERNAME_DOES_NOT_FIT_PATTERN" => "Username does not fit the name scheme: only a-Z and numbers are allowed, 2 to 64 characters.",
+	"FEEDBACK_USERNAME_DOES_NOT_FIT_PATTERN" => "Username does not fit the name pattern: only a-Z and numbers are allowed, 2 to 64 characters.",
 	"FEEDBACK_EMAIL_DOES_NOT_FIT_PATTERN" => "Sorry, your chosen email does not fit into the email naming pattern.",
 	"FEEDBACK_EMAIL_SAME_AS_OLD_ONE" => "Sorry, that email address is the same as your current one. Please choose another one.",
 	"FEEDBACK_EMAIL_CHANGE_SUCCESSFUL" => "Your email address has been changed successfully.",
 	"FEEDBACK_CAPTCHA_WRONG" => "The entered captcha security characters were wrong.",
 	"FEEDBACK_PASSWORD_REPEAT_WRONG" => "Password and password repeat are not the same.",
 	"FEEDBACK_PASSWORD_TOO_SHORT" => "Password has a minimum length of 6 characters.",
 	"FEEDBACK_USERNAME_TOO_SHORT_OR_TOO_LONG" => "Username cannot be shorter than 2 or longer than 64 characters.",
-	"FEEDBACK_EMAIL_TOO_LONG" => "Email cannot be longer than 64 characters.",
 	"FEEDBACK_ACCOUNT_SUCCESSFULLY_CREATED" => "Your account has been created successfully and we have sent you an email. Please click the VERIFICATION LINK within that mail.",
 	"FEEDBACK_VERIFICATION_MAIL_SENDING_FAILED" => "Sorry, we could not send you an verification mail. Your account has NOT been created.",
 	"FEEDBACK_ACCOUNT_CREATION_FAILED" => "Sorry, your registration failed. Please go back and try again.",
@@ -45,6 +44,9 @@
 	"FEEDBACK_AVATAR_UPLOAD_TOO_BIG" => "Avatar source file is too big. 5 Megabyte is the maximum.",
 	"FEEDBACK_AVATAR_FOLDER_DOES_NOT_EXIST_OR_NOT_WRITABLE" => "Avatar folder does not exist or is not writable. Please change this via chmod 775 or 777.",
 	"FEEDBACK_AVATAR_IMAGE_UPLOAD_FAILED" => "Something went wrong with the image upload.",
+	"FEEDBACK_AVATAR_IMAGE_DELETE_SUCCESSFUL" => "You successfully deleted your avatar.",
+    "FEEDBACK_AVATAR_IMAGE_DELETE_NO_FILE" => "You don't have a custom avatar.",
+    "FEEDBACK_AVATAR_IMAGE_DELETE_FAILED" => "Something went wrong while deleting your avatar.",
 	"FEEDBACK_PASSWORD_RESET_TOKEN_FAIL" => "Could not write token to database.",
 	"FEEDBACK_PASSWORD_RESET_TOKEN_MISSING" => "No password reset token.",
 	"FEEDBACK_PASSWORD_RESET_MAIL_SENDING_ERROR" => "Password reset mail could not be sent due to: ",
@@ -54,10 +56,7 @@
 	"FEEDBACK_PASSWORD_RESET_LINK_VALID" => "Password reset validation link is valid. Please change the password now.",
 	"FEEDBACK_PASSWORD_CHANGE_SUCCESSFUL" => "Password successfully changed.",
 	"FEEDBACK_PASSWORD_CHANGE_FAILED" => "Sorry, your password changing failed.",
-	"FEEDBACK_ACCOUNT_UPGRADE_SUCCESSFUL" => "Account upgrade was successful.",
-	"FEEDBACK_ACCOUNT_UPGRADE_FAILED" => "Account upgrade failed.",
-	"FEEDBACK_ACCOUNT_DOWNGRADE_SUCCESSFUL" => "Account downgrade was successful.",
-	"FEEDBACK_ACCOUNT_DOWNGRADE_FAILED" => "Account downgrade failed.",
+	"FEEDBACK_ACCOUNT_TYPE_CHANGE_SUCCESSFUL" => "Account type change successful",
 	"FEEDBACK_ACCOUNT_TYPE_CHANGE_FAILED" => "Account type change failed",
 	"FEEDBACK_NOTE_CREATION_FAILED" => "Note creation failed.",
 	"FEEDBACK_NOTE_EDITING_FAILED" => "Note editing failed.",

diff --git a/application/controller/LoginController.php b/application/controller/LoginController.php
@@ -135,13 +135,13 @@ public function editUserEmail_action()
     }
 
     /**
-     * Upload avatar
+     * Edit avatar
      * Auth::checkAuthentication() makes sure that only logged in users can use this action and see this page
      */
-    public function uploadAvatar()
+    public function editAvatar()
     {
         Auth::checkAuthentication();
-        $this->View->render('login/uploadAvatar', array(
+        $this->View->render('login/editAvatar', array(
             'avatar_file_path' => AvatarModel::getPublicUserAvatarFilePathByUserId(Session::get('user_id'))
         ));
     }
@@ -155,38 +155,50 @@ public function uploadAvatar_action()
     {
         Auth::checkAuthentication();
         AvatarModel::createAvatar();
-        Redirect::to('login/uploadAvatar');
+        Redirect::to('login/editAvatar');
+    }
+
+    /**
+     * Delete the current user's avatar
+     * Auth::checkAuthentication() makes sure that only logged in users can use this action and see this page
+     */
+    public function deleteAvatar_action()
+    {
+        Auth::checkAuthentication();
+        AvatarModel::deleteAvatar(Session::get("user_id"));
+        Redirect::to('login/editAvatar');
     }
 
     /**
      * Show the change-account-type page
      * Auth::checkAuthentication() makes sure that only logged in users can use this action and see this page
      */
-    public function changeAccountType()
+    public function changeUserRole()
     {
         Auth::checkAuthentication();
-        $this->View->render('login/changeAccountType');
+        $this->View->render('login/changeUserRole');
     }
 
     /**
      * Perform the account-type changing
      * Auth::checkAuthentication() makes sure that only logged in users can use this action
      * POST-request
      */
-    public function changeAccountType_action()
+    public function changeUserRole_action()
     {
         Auth::checkAuthentication();
 
         if (Request::post('user_account_upgrade')) {
             // "2" is quick & dirty account type 2, something like "premium user" maybe. you got the idea :)
-            AccountTypeModel::changeAccountType(2);
+            UserRoleModel::changeUserRole(2);
         }
+
         if (Request::post('user_account_downgrade')) {
             // "1" is quick & dirty account type 1, something like "basic user" maybe.
-            AccountTypeModel::changeAccountType(1);
+            UserRoleModel::changeUserRole(1);
         }
 
-        Redirect::to('login/changeAccountType');
+        Redirect::to('login/changeUserRole');
     }
 
     /**

diff --git a/application/core/Application.php b/application/core/Application.php
@@ -26,18 +26,8 @@ public function __construct()
         // create array with URL parts in $url
         $this->splitUrl();
 
-        // check for controller: no controller given ? then make controller = default controller (from config)
-        if (!$this->controller_name) {
-            $this->controller_name = Config::get('DEFAULT_CONTROLLER');
-        }
-
-        // check for action: no action given ? then make action = default action (from config)
-        if (!$this->action_name OR (strlen($this->action_name) == 0)) {
-            $this->action_name = Config::get('DEFAULT_ACTION');
-        }
-
-        // rename controller name to real controller class/file name ("index" to "IndexController")
-        $this->controller_name = ucwords($this->controller_name) . 'Controller';
+	    // creates controller and action names (from URL input)
+	    $this->createControllerAndActionNames();
 
         // does such a controller exist ?
         if (file_exists(Config::get('PATH_CONTROLLER') . $this->controller_name . '.php')) {
@@ -87,4 +77,24 @@ private function splitUrl()
             $this->parameters = array_values($url);
         }
     }
+
+	/**
+	 * Checks if controller and action names are given. If not, default values are put into the properties.
+	 * Also renames controller to usable name.
+	 */
+	private function createControllerAndActionNames()
+	{
+		// check for controller: no controller given ? then make controller = default controller (from config)
+		if (!$this->controller_name) {
+			$this->controller_name = Config::get('DEFAULT_CONTROLLER');
+		}
+
+		// check for action: no action given ? then make action = default action (from config)
+		if (!$this->action_name OR (strlen($this->action_name) == 0)) {
+			$this->action_name = Config::get('DEFAULT_ACTION');
+		}
+
+		// rename controller name to real controller class/file name ("index" to "IndexController")
+		$this->controller_name = ucwords($this->controller_name) . 'Controller';
+	}
 }
diff --git a/application/core/Config.php b/application/core/Config.php
@@ -2,15 +2,22 @@
 
 class Config
 {
-    private static $config;
+	// this is public to allow better Unit Testing
+    public static $config;
 
     public static function get($key)
     {
         if (!self::$config) {
-            self::$config = require('../application/config/config.' . Environment::get() . '.php');
+
+	        $config_file = '../application/config/config.' . Environment::get() . '.php';
+
+			if (!file_exists($config_file)) {
+				return false;
+			}
+
+	        self::$config = require $config_file;
         }
 
         return self::$config[$key];
     }
-
 }
diff --git a/application/core/Mail.php b/application/core/Mail.php
@@ -67,10 +67,7 @@ public function sendMailWithPHPMailer($user_email, $from_email, $from_name, $sub
 			$mail->Username = Config::get('EMAIL_SMTP_USERNAME');
 			$mail->Password = Config::get('EMAIL_SMTP_PASSWORD');
 			$mail->Port = Config::get('EMAIL_SMTP_PORT');
-		}
-
-		// if you want to send mail via PHPMailer using native mail()
-		if (!Config::get('EMAIL_USE_SMTP')) {
+		} else {
 			$mail->IsMail();
 		}
 
@@ -81,15 +78,16 @@ public function sendMailWithPHPMailer($user_email, $from_email, $from_name, $sub
 		$mail->Subject = $subject;
 		$mail->Body = $body;
 
-		// send mail
+		// try to send mail
 		$mail->Send();
+
 		if ($mail) {
 			return true;
+		} else {
+			// if not successful, copy errors into Mail's error property
+			$this->error = $mail->ErrorInfo;
+			return false;
 		}
-
-		// if not successful, copy errors into Mail's error property
-		$this->error = $mail->ErrorInfo;
-		return false;
 	}
 
 	public function sendMail($user_email, $from_email, $from_name, $subject, $body)

diff --git a/application/core/Text.php b/application/core/Text.php
@@ -6,10 +6,21 @@ class Text
 
     public static function get($key)
     {
+	    // if not $key
+	    if (!$key) {
+		    return null;
+	    }
+
+	    // load config file (this is only done once per application lifecycle)
         if (!self::$texts) {
             self::$texts = require('../application/config/texts.php');
         }
 
+	    // check if array key exists
+	    if (!array_key_exists($key, self::$texts)) {
+		    return null;
+	    }
+
         return self::$texts[$key];
     }