Last week we finished our first ever dedicated foodsharing.de hackweek!

We tried out the new week long format last September at our foodsaving worldwide hackweek and found it gave us more time to focus on more substantial/abstract/architectural changes.

We were a small team - with me, Matthias, and Tilmann focused for the whole duration, with shorter visits from other people:

  • Kristijan came and implemented the new communities overview pages with our new contributor Basti
  • Jörg came to meet the dev team and have some discussions about data protection topics
  • Chandi came and implemented a more secure password hashing algorithm (using Argon2i).
  • Basti came and fixed some requested issues like not listing the work groups on the details page reachable through the team page.

Tilmann, Nick, and Matthias discussing some of the finer points of PHP

Matthias, Chandi, Kristijan, and Tilmann in the repurposed silent working office at kanthaus

We started with a meeting to discuss our goals and expectations for the week. Tilmann was focused on improving the developer experience, Matthias wanted to work on refactoring but also some user-facing improvements, and external contact, I was torn between coding tasks, and wider topics that concern the project.

We progressed through our Incremental refactor issue with our guiding Modernizing Legacy Applications In PHP book.

Matthias took on the task of removal global variables from the codebase, to give you a better idea of the task, there might be code like this (code examples borrowed from the book):

class Example
{
    public function fetch()
    {
        global $db;
        return $db->query(...);
    }
}

The problem here is we don’t know what sets $db (or whether it is set at all).

The first step is to move it to the constructor:

class Example
{
	protected $db;
	public function __construct()
	{
		global $db;
		$this->db = $db;
	}
	public function fetch()
	{
		return $this->db->query(...);
	}
}

This might not look very different, but it’s a step towards making the dependencies explicit.

I jumped to a later step in the book to add the Symfony Dependency Injection (DI) container, which meant we could jump to the final goal:

class Example
{
	protected $db;
	public function __construct(Database $db)
	{
		$this->db = $db;
	}
	public function fetch()
	{
		return $this->db->query(...);
	}
}

This means we don’t need to think about how to instantiate the Database object, and it can be reused by any other instance too.

Templating

The existing way of templating was via composable functions and string concatenation in PHP, e.g.

$out = '
<ul id="teamlist" class="linklist">';

foreach ($team as $t) {
	$socials = '&nbsp;';
	if ($t['homepage'] != '') {
		$socials .= '<i class="fa fa-globe"><span>' . $t['homepage'] . '</span></i>';
	}

	if ($t['twitter'] != '') {
		$socials .= '<i class="fa fa-twitter"><span>' . $t['twitter'] . '</span></i>';
	}

	if ($t['github'] != '') {
		$socials .= '<i class="fa fa-github"><span>' . $t['github'] . '</span></i>';
	}

	if ($t['tox'] != '') {
		$socials .= '<i class="fa fa-lock"><span>' . $t['id'] . '</span></i>';
	}

	$out .= '
	<li>
		<a id="t-' . $t['id'] . '" href="/team/' . $t['id'] . '" class="corner-all" target="_self">
			<span class="img" style="background-image:url(/images/q_' . $t['photo'] . ');"></span>
			<h3>' . $t['name'] . ' ' . $t['nachname'] . '</h3>
			<span class="subtitle">' . $t['position'] . '</span>
			<span class="desc">
				' . $this->func->tt($t['desc'], 240) . '
			</span>
			<span class="foot corner-bottom">
				' . $socials . '	
			</span>
		</a>
		' . $this->toxPopTpl($t) . '
	</li>';
}

$out .= '
</ul>';

This has worked, but suffers from a few difficulties:

  • it is not escaped by default (easy to accidently cause XSS vulnerability)
  • editor support is not available as it just sees a string
  • it’s sometimes tricky to combine html/css/js/php and get the quoting/variables correct
  • data access and templating are combined are hard to reason about (any part of the view might access the databse)

We moved to gathering the data required first, then passing it to a twig template, e.g.:

$params = [
	'loggedIn' => $loggedIn,
	'fsId' => $fsId,
	'image' => $image,
	'mailbox' => $sessionMailbox,
	'hasFsRole' => $hasFsRole,
	'isOrgaTeam' => $isOrgaTeam,
	'may' => [
		'editBlog' => $mayEditBlog,
		'editQuiz' => $mayEditQuiz,
		'handleReports' => $mayHandleReports
	],
	'stores' => $stores,
	'regions' => $regions,
	'workingGroups' => $workingGroups
];

return [
	'default' => $this->twig->render('partials/menu.default.twig', $params),
	'mobile' => $this->twig->render('partials/menu.mobile.twig', $params)
];

The nice thing about this approach is we can incrementally change one function at a time.

We also switched this to use twig templates for the layouts, and reduced the repetition between the three in use (default, map, and message layouts).

This means layouts can inherit from each other, and just modify the parts they need to. For example, the new map layout looks like this:

{% extends 'layouts/default.twig' %}

{% block head %}
    <link rel="stylesheet" href="/js/markercluster/dist/MarkerCluster.css" />
    <link rel="stylesheet" href="/js/markercluster/dist/MarkerCluster.Default.css" />
{% endblock %}

{% block full %}
    {{ content.main.html | raw }}
    {{ content.top.html | raw }}
{% endblock %}

{% block footer %}
{% endblock %}

These two approaches address templating from each end: layouts (the main frame of the site), and partials (the small pieces of the site).

We also managed the first fully twig rendered view for the WorkGroup list page, which involved a totally new structure for rendering responses.

The existing approach was to match URLs with methods, the methods then fetch data add bits of content (html, css, js) to global variables which then get combined to form the response.

With the new twig templating approach, the logic and data acccess is seperated from the template rendering.

We also added Symfony Request and Response objects to move away from using PHP superglobals, and closer to a standard Symfony controller approach.

The general theme is inverting the architecture from having global structures that control the flow of a request, to localized control where a controller is in full control of the response (but can access shared data/functions/templates where needed).

Database access

Currently the access to the database is quite hard to reason about, the Model classes have quite a deep hierarchy and there are lots of very similar methods, many unused.

The new approach is to define database Gateway classes to abstract databaes access, they are similar to the Model classes, except:

  • use PDO instead of mysqli
  • have (almost) no class hierarchy (prefer composition)
  • throw errors instead of returning false
  • return empty arrays when no results instead of false
  • use prepared statements instead of string concatenation
  • use nice helper methods that take objects, over raw SQL

For example, instead of using SQL with string concatenation we can use the update helper of the Database class that takes objects, and internally will use prepared statements:

public function stickThread($thread_id)
{
	return $this->db->update(
		'fs_theme',
		['sticky' => 1],
		['id' => $thread_id]
	);
}

Developer Experience improvements

Developer Experience is really important to us, and Tilmann made some great improvements in this area, notable:

Improved seed data

When you start up the development environment there are a few user accounts to log in as, but there was almost no data in the site to look at (e.g. forum posts, pickups, stores, other users, etc).

Now there is a whole nice set of reasonably realistic looking data - especially if you can’t understand German ;)

Nicely populated site running locally

The data is generated by reusing and extending the helpers we use for the tests, e.g.:

$fairteiler = $I->createFairteiler($userbot['id'], $bezirk1);
$I->addFairteilerFollower($user2['id'], $fairteiler['id']);
$I->addFairteilerPost($userbot['id'], $fairteiler['id']);

// create users and collect their ids in a list
$this->foodsavers = [$user2['id'], $userbot['id'], $userorga['id']];
foreach (range(0, 100) as $_) {
	$user = $I->createFoodsaver('user', ['bezirk_id' => $bezirk1]);
	$this->foodsavers[] = $user['id'];
	$I->addStoreTeam($store['id'], $user['id']);
	$I->addCollector($user['id'], $store['id']);
	$I->addStoreNotiz($user['id'], $store['id']);
	$I->addForumThemePost($theme['id'], $user['id']);
}

// create conversations between users
foreach ($this->foodsavers as $user) {
	foreach ($this->getRandomUser(10) as $chatpartner) {
		if ($user !== $chatpartner) {
			$conv = $I->createConversation([$user, $chatpartner]);
			$I->addConversationMessage($user, $conv['id']);
			$I->addConversationMessage($chatpartner, $conv['id']);
		}
	}
}

Better handling of test errors

Most of our tests are full end to end acceptance tests (simulating clicks within a real browser, database, etc), and these type of tests are notoriously flakey.

It’s also the type of issue that is really unmotivating to work on (as it is not predictable in which way it will fail, and has a long cycle to see the results of any changes).

Tilmann dug deep into Codeception and improved the reliability of some tests. He also had a great idea to simply retry failed tests. They often work the next time around (and if it works the second time it is most likely that the test was at fault, not the code).

This saves a lot of time and frustration :)

Here we can see test failures, but when re-tried they pass!

Many other things

We also did many other smaller things. See the CHANGELOG for all the details.

One of my favourite things is we now use GitLab environments to keep track of exactly which commit is deployed to beta and production.

Overview of the environments, plus button to rollback to previous version

Helpfully includes the information in the merge request view

So, how was it all?

We really enjoyed the event. It’s a rare occasion that we get to focus fully on the project and Matthias particularly appreciated having other people contributing more actively.

It was a great opportunity to use the space at kanthaus and the hackweek was fully supported by the house and it’s inhabitants - meaning lots of food prepared for us :)

We eagerly await the next hackweek!

You?

If this is the kind of thing you think you’d like to work on, come and chat to us in the #foodsharing-dev channel in yunity slack!

We have quite a job now to move the whole codebase over to using these new approaches… also keen to start tackling the frontend/javascript too…