[ticket/17025] Move post destination topic field should not be populated with a zero

[battye]

Move post destination topic field should not be populated with a zero (via @stevemaury)

PHPBB3-17025

Checklist:

• Correct branch: master for new features; 3.3.x for fixes
• Tests pass
• Code follows coding guidelines: master and 3.3.x
• Commit follows commit message format

Tracker ticket:

https://tracker.phpbb.com/browse/PHPBB3-17025

[battye]

@marc1706 What do you think about when the #6451 Codespaces change goes live in 3.3.11, we have some documentation in the release discussion topic to show people how it can be used to contribute to the project? What I'm thinking of is written below as I thought this being a simple PR/bug fix might be a good way to demonstrate how it works.

---

Background

A GitHub Codespace is a pre-configured cloud development environment. It includes a web-based code editor (VS Code) as well as a virtual machine which can be used to run software. The primary benefit of using a GitHub Codespace is that a code change can be made online from any computer without the need for a local development environment.

For phpBB, the pre-configured Codespace contains a LAMP stack with XDebug - allowing developers to modify and debug a vanilla board which is pre-installed.

This tutorial explains how to:

• Create a GitHub Codespace with an automatically created fresh installation of phpBB
• Use a Codespace to make a code change to phpBB
• Use XDebug to debug phpBB code in real time

Step 1

See if there is a ticket in the phpBB issue tracker describing the problem you would like to fix or the feature you would like to add. If there is not an existing ticket, you can create a new ticket, logging in using your phpBB.com login details.

In this case, we'll look at a simple bug that was raised at https://tracker.phpbb.com/projects/PHPBB3/issues/PHPBB3-17025 - the number 0 appears as the default topic ID on the MCP move posts page, but there is no valid use case for this because it is an invalid ID. We will use this bug to demonstrate how GitHub Codespaces works and how it can be used for phpBB development.

The reporter provided some valuable information which acts as a good starting point, namely that the bug is on line 76 of prosilver/template/mcp_topic.html. We will come back to this in Step 3.

Step 2

Create a new branch on your own fork of phpBB. If you have never done this before, the procedure is outlined at https://area51.phpbb.com/docs/dev/3.3.x/development/git.html

Then on GitHub.com, create a new Codespace for this branch. It will already be preconfigured from the code in the phpbb/phpbb repository.

This will create a brand new installation of phpBB with XDebug installed to assist developers.

Step 3

From the online VS Code website, which looks almost identical to the desktop version of VS Code, click on the files icon and navigate to mcp_topic.html - or use the shortcut Ctrl+P to search for the file.

By looking at line 76 of that file, we can see that the "value" attribute listed in the ticket is set to a variable called TO_TOPIC_ID.

Using the Ctrl+Shift+F (global search) feature, if we search for that TO_TOPIC_ID variable we can see that it only gets used in one other place.

Double click mcp_topic.php to open the other place where that variable is used - line 386 of the file.

The template variable is set to take the exact value of $to_topic_id.

Step 4

Using the power of XDebug, we can debug the issue in real time. Using the Ctrl+F (local search) feature, we can see where $to_topic_id is declared and how it is used.

On line 52 it takes input from the URL - and defaults to 0 if the parameter isn't supplied in the URL. On line 357, it is forcibly set to 0 if a certain condition is met.

Line 376 is the best place to see XDebug in action though. Left click once just to the side of the line number so that a red breakpoint appears. A breakpoint is used to identify where execution should temporarily halt during debugging.

Step 5

Now that a breakpoint has been set (you can set as many breakpoints as you wish), debugging can be turned on by clicking the "Run and Debug" button and then the green play button beside "Debug phpBB".

Step 6

At this point, debug mode is active. Under the Ports tab at the bottom of screen, click the "Open in Browser" button and the fresh phpBB installation will open.

Log in to the board with the credentials admin/adminadmin and, for the purposes of this example, add some posts and topics so that posts can be moved in the MCP to replicate the original bug.

Step 7

When the move posts page of the MCP is loaded, the XDebug breakpoint will automatically be hit - meaning the execution of the page is paused at the point to give developers the chance to analyse the code before it has finished running.

The tab will show a small red circle to indicate a breakpoint has been hit. Within VS Code, the line will be highlighted as well.

To identify the value of the variables at the point the execution has paused, either hover over a variable (like in the case of $to_topic_id on the right of screen) or look at the variable list on the left of screen.

Step 8

From this information, we learn that $to_topic_id is set to 0 as a default and it has been done intentionally. So to fix the bug of 0 appearing by default in the template, we don't want to refactor the entire file and potentially introduce a new problem.

The sensible approach is to just change the way the value is passed to the template because, as we discovered from the global search in Step 3, the template variable does not get used anywhere else.

Using the ternary operator we can make a simple change so that the template variable declaration reads as:

'TO_TOPIC_ID'		=> $to_topic_id ?: '',

This means that if $to_topic_id evaluates to true (the integer 0 does not evaluate to true, but if a valid topic ID is passed through the URL as explained in Step 4 it does evaluate to true), then $to_topic_id will be passed as the default value to the input field - otherwise, an empty string will.

Step 9

By returning to the tab with the phpBB board open, we can confirm that the fix is working as intended.

Step 10

Satisfied that the fix is correct, we can now commit and push the change to the local branch. This is easily done in VS Code, simply click the "Source Control" tab and stage the change to mcp_topic.php (the file we edited) by right clicking it and clicking "Stage Changes".

Type a commit message in the box (using the format described at https://area51.phpbb.com/docs/dev/3.3.x/development/git.html), and then select "Commit and Push" from the dropdown option.

The list of files in the install directory can be safely ignored - they are deleted when setting up the Codespace to allow phpBB to run normally.

Step 11

Finally, create a pull request on GitHub.com from your local branch to either the phpbb/phpbb 3.3.x branch or the master branch. phpBB's GitHub Actions will run unit tests, ensuring no features were broken accidentally as a result of the change, and then the development team will review the code.

[marc1706]

How about actually creating a blog post about this which we can link to from the announcement as well?

[battye]

How about actually creating a blog post about this which we can link to from the announcement as well?

I think that's a good idea

[marc1706]

@battye Let's prepare the blog post sooner than later. We can also post the blog post before and then refer to it again in the announcement.