lewsid/dinkly

The biggest little PHP framework

dinkly's Issues

initWith() not returning true if record has been found

In BaseDinklyDataModel.

Need to add a return true; after $this->hydrate() in the initWith function.

loadModule App Jumping Issue

loadModule can't handle jumping across apps when the controllers are named the same in more than one app. Cannot redeclare class.

Warnings thrown on gen_models.php

Warnings are thrown on gen_models.php for plugins that don't have a config/schemas folder.

Update Bootstrap Version

Still using version 2...

Implement unit testing

PHPUnit vs SimpleTest?

Incorporate page titles handling

I've gotten used to setting dynamic page titles in the Dinkly class, but there has to be a better way.

PHP 7.2 Function create_function() is deprecated

In numerous places in BaseDinkly.php

PHP 7.2 Function create_function() is deprecated

ErrorHttpController inherits from Dinkly, not ErrorController

Table indexes

Add support for table indexes

gen_model.php not adding missing columns

This works fine when running the blanket gen_models.php tool, but not for the singular gen_model.php

Unable to use built in DinklyDataModel->update with models that do not use 'id' as primary key

This is easy enough to get around by overriding the call to update and adjusting the final line of the query build:
$query .= " where id='" . $this->Id . "'";
on line 132 of DinklyDataModel to what you need. But just wanted to give you a heads up. Possibly allow specification of (compound) primary key in the config schema for the model?

Refactor Application name handling in loadModule

Based on conversations with Joe, refactor session app name handling:

$_SESSION['app_name']['app_name']

Perhaps by extending the capabilities of AppControllers to be aware of the app name.

Extend ORM to handle Joins

@bigSiebs:

I guess what I find lacking is that the ORM support doesn’t an easy way to query against related tables. I get not wanting to hydrate all the joined data, but being able to stick a related field in the where clause (and then the ORM would implicitly add a join to the query) and then only returning/hydrating one model would save me from having to write 75% of the raw queries I end up writing.

Or even just getWith, but with a table_name.field_name convention. The schema would have to be aware of these relationships to make something like that happen, though. I don’t think it would be a huge lift, and I can imagine ways that it could be layered on top of what we have know without breaking anything.

Update module not found exception

Update error output to suggest user try '?nocache=1'

Additional API headers

Improve support for a wider range of API headers - ie, for specific error states.

Containerize components

loadComponent should be isolating it's context to prevent collision.

gen_module says success when unable to create module

In BaseDinklyBuilder, the buildModule() function echoes the success message even when if(mkdir($module_folder)) throws a warning. This happened when I tried to run gen_module and there was no module folder in the app directory.

Break up Dinkly::loadModule

It would be good to have a redirect method that handles redirecting the user to a view based on app, module, view, etc.

The other method would handle locating the controller method to execute and calling it.

Controllers should inherit from abstract Controller class, not Dinkly

It's very hard to reason about where controller functionality comes from because Dinkly instantiates controller objects, and those controller objects also inherit from Dinkly. I think it'd be better to separate Dinkly and controllers, and then have the latter inherit from an abstract controller class:

abstract class Controller
{
	private $app;

	private $module;

	public function getApp()
	{
		// Gets app
	}

	public function getModule()
	{
		// Gets module
	}

	public function json($data, int $status = 200, array $headers = [])
	{
		// Returns JsonResponse object
	}

	public function file($file, string $fileName = null, string $disposition = ResponseHeaderBag::DISPOSITION_ATTACHMENT)
	{
		// Returns BinaryFileResponse object
	}

	public function render(bool $with_layout = true)
	{
		// Returns Response object with HTML content
	}
}

Couple things to figure out:

1. We have app-level controllers that don't have modules. Does it make sense to add this everywhere?
2. How do we actually implement the render method? That's currently happening with Dinkly, and relies on some on it's context.

I think it's important that this class be extensible in some way. Like if I wanted to start using a template engine like twig, I could create a twig method somewhere that would make it available on all controllers.

Automatically populate default values when creating new model

It'd be nice if, when creating a new model instance, any default values that are set on insert are also set on the model instance.

For example, say we have the following config file:

table_name: dinkly_user
registry:
  - id
  - username: { type: varchar, length: 64, allow_null: false }
  - is_cool: { type: boolean, length: 1, allow_null: false, default: 1}

Then, somewhere in the code, we went to used the generated model for this table.

$user = new User($db);
$user->setUsername('CoolGuy1');
$user->save();

After saving, the $user object's id property has been set, but the isCool property is null, even though the record's field is set to 1 in the DB. It'd be nice if the model's property was automatically set to 1.

I think this could be implemented in one of two ways.

1. Add another property to the automatically generated base-prefixed model classes. You could call it $defaults. Then, when constructing a new model instance, any properties listed in this array would automatically be set equal to the default value.

2. Backfill the model after inserting. I'm not sure if there's a more efficient way to do it, but after inserting the record, you could run a select query using the lastInsertId and then populating the values into the model.

Add option to specify database name for load_fixtures

Currently, it will default to dinkly if there is more than one database.

Add LICENSE.md

We need a LICENSE.md and this may be the appropraite contents: https://opensource.org/licenses/MIT

MySQL Keyword Validation

Using the word 'key' for a table name in a model's YAML causes problems. Adding a filter for such things directly to Dinkly would be very cool

Let's add $parameters in loadDefault on generated modules

Instead of:

public function loadDefault()

Let's have it include parameters:

public function loadDefault($parameters)

I think it is a better example out of the gate of usage that way.

Get deprecation warning when running gen_module.php in PHP 7.2

PHP Deprecated: Function create_function() is deprecated in /Users/joe/Dropbox (Bluehouse Group)/projects/path/classes/core/custom/Dinkly.php on line 597

That's in a override of the convertToCamelCase in BaseDinkly. The overridden function would throw the same notice.

Plugin integration

Devise a solution for integrating Dinkly-specific plugins.

Handle PDOException: SQLSTATE[HY000] [2002] more elegantly

You run into this error when you try to run gen_models.php without having first run these commands on your machine:

sudo mkdir /var/mysql
sudo ln -s /Applications/MAMP/tmp/mysql/mysql.sock /var/mysql/mysql.sock

It would be nice if we handled the exception and suggested running these commands. We almost always have to do this when setting up new machines, and that's so infrequent that it's hard to remember that you have to run these two.

Additional Composer support

Default values

Add attribute for setting default values to schema files.

Convert apps into classes

It would be nice to have a class to represent each app, so that app-wide tweaks can be applied to all modules beneath it.

Fix setting zero values

Update config.yml when creating new app

When using app gen tool, also add configs to config.yml

Cleanup $_SERVER['APPLICATION_ROOT'] and fix double slash

I can do this or create a pull request if you like.

There are two issues with $_SERVER['APPLICATION_ROOT']. The first is mostly cosmetic I guess.

We have this in bootstrap.php.
$_SERVER['APPLICATION_ROOT'] = dirname(__FILE__) . '/../';
The resulting path ends in /config/../ and any calculated path contains /config/../ .
If nothing else, this just looks untidy when debugging.

I suggest changing it to
$_SERVER['APPLICATION_ROOT'] = realpath(dirname(__FILE__) . '/..');

That produces a simple clean path like /var/apps/myapp rather than /var/apps/myapp/config/../

The other issue is that $_SERVER['APPLICATION_ROOT'] is used inconsistently in terms of whether or not a trailing slash is expected. In the many (most?) places, the appended path adds another slash so there are many places where the resulting path is calculated with a double slash like /var/apps/myapp/config/..//somepath. This seems to work but is untidy and I suspect might even be ambiguous on some file systems.

If we use my realpath suggestion above, we need to find all uses of $_SERVER['APPLICATION_ROOT'] and make sure that a slash is added or alternatively make the bootstrap line this:
$_SERVER['APPLICATION_ROOT'] = realpath(dirname(__FILE__) . '/..') . '/';
and remove the slash from appended paths. Either way is okay but we should do one or the other.

PHP7: Array to string conversion errors in BaseDinklyDataModel

Loading any page that requires a data model will throw Array to String conversion notices and eventually crash

This is because of a new variable syntax.

                        // old meaning            // new meaning
$foo->$bar['baz']       $foo->{$bar['baz']}       ($foo->$bar)['baz'] 

In hydrate(), update() and insert(), anything using the $foo->$bar[$property] syntax has to be updated to $foo->{$bar[$property]}

Warning related to Dinkly -> filterFiles

PHP Warning: Missing argument 1 for BaseDinkly::filterFiles(), called in /Users/joe/Dropbox (Bluehouse Group)/projects/path/classes/core/base/BaseDinkly.php on line 943 and defined in /Users/joe/Dropbox (Bluehouse Group)/projects/path/classes/core/base/BaseDinkly.php on line 736

I think the argument should be removed from this function's signature. I don't see any indication that it should be passed something.

$db argument

In BaseDinklyDataCollection::getWith() we have added $db as the last argument, but this will look funky as we add other arguments. I propose we move it to the first argument and update all of our codebases that use this.

Block any attempts to include ../ in the incoming URL

The module name part of the URL causes a PHP controller file to be included. We should block any attempts to navigate around the file system by requesting a path with ../../ etc. It's probably a stretch because the filename needs to end in Controller.php but even so, it's an easy hole to patch in BaseDinkly::getContext. It could potentially be an issue if coupled with something like a careless image upload which allowed a PHP file to be uploaded.

Not specifying a length for a varchar field in the .yml generates invalid sql

In schema .yml files if no length parameter is specified for a type that requires it, invalid sql is generated, but no error is thrown for the user that specifies what to fix or what went wrong.

Calling save on a model after hydrating without specifying second parameter throws non-descript error

Hydrate's second parameter, $hasDB, defaults to false, and if this is the case and you run a save on the model, the error:
Call to a member function quote() on a non-object in...
is thrown. Possibly check that $this->db is not null when save is called, and if it is, throw a more meaningful exception?

Ability to add foreign keys from table schema