Modify per-language user list page to make it easier to find active native speakers

[franklindyer]

fixes #3075

• adds an icon to users on the per-language user list page (at the endpoints /users/for_language/XXX) to users who have been active within the past week
• also sorts users on the per-language user list page by the time of their most recent sentence/translation

This should make it easier to find the most active / likely-to-respond users in a given language on Tatoeba. See this former PR for further discussion.

It would be nice to get some feedback from @alanfgh on whether this would meet his need. Alternatively, perhaps this could get rolled out to the dev instance of Tatoeba and we could ask alanfgh to try it out.

[franklindyer]

It looks like the CI is failing because the users table that is being used for Jenkins testing lacks the new last_contribution column. Is there something that needs to be changed in the repo to make the tests go through, or is there some database used for testing on a machine external to the repo?

Fyi, this PR includes a script docs/database/updates/2025-10-13.sql for adding the new column to the users table.

[jiru]

To fix the CI, you can edit the fixture file tests/Fixture/UsersFixture.php. You need to edit both the schema definition and the table data. You can edit the file manually, or use the cake console with cake bake fixture (from within the VM, from the Tatoeba repository), or a combination of both.

To run the tests locally, you can run phpunit. This will help you confirm the CI should pass without having to git push your changes.

To add the new column, we used to create migration scripts in docs/database/updates/, but this is no longer the way we do it now. Instead, you should create a migration script with cake bake migration, and then execute it with cake migrations migrate. This method allows you to also rollback with cake migrations rollback, as well as execute custom code, typically to populate the new field for existing users.

The table definition in docs/database/tables/users.sql is what the table looked like before we started using migration scripts, so you need not to edit it. Sorry this is all very confusing!

[jiru]

CakePHP offers a mechanism to perform some action on every creation or update of a model: the afterSave callback. Have a look at the afterSave() method in UsersTable.php or SentencesTable.php. You could call update_user_last_contribution() from there instead. The advantage is that:

• you call it only once, instead of calling it from every controller action potentially involved in sentence creation/update
• it is easier to write unit tests

[franklindyer]

Thanks for the pointers! The unit tests are now passing, and I moved update_user_last_contribution to a similarly named method in UsersTable.php which gets called in the afterSave method of SentencesTable.php.

[alanfgh]

It would be nice to get some feedback from @alanfgh on whether this would meet his need. Alternatively, perhaps this could get rolled out to the dev instance of Tatoeba and we could ask alanfgh to try it out.

It does sound like this approach would accomplish what I was looking for. In any case, if you move your changes over to dev, let me know, and I'll check it out there. Thanks!

[jiru]

This file can be removed now.

[jiru]

I think it would be better to use the datetime type rather than integer. This will let CakePHP automatically interpret it as a datetime, providing type check and datetime calculation methods. Using integer also makes your implementation vulnerable to the year 2038 bug.

[franklindyer]

Sounds good. I only used the integer type in order to be consistent with the last_time_active field.

[jiru]

You can use the following pattern instead:

use Cake\Datasource\Exception\RecordNotFoundException;

try {
    $user = $this->get($userId);
} catch (RecordNotFoundException $e) {
}

The benefit is that only one SQL SELECT request is performed (at get()) instead of two (both at exists() and get()).

[jiru]

There are two problems with this icon.

1. The text Active this week is not translatable, you can use __("Active this week") to make it so. Also wrap with h() to avoid any quote returned by __() to mess up with your markup.
2. The HTML title attribute should be avoided because of its accessibility concerns. You can use <md-tooltip> instead, see the documentation and search around the codebase for some usage more examples.

[franklindyer]

Got it. I re-ran generate_pot.sh as well to update the localization files... let me know if you see any issues with this.

[jiru]

That’s really not a big deal, but the position of the last_contribution column is different here than what you specified in the fixtures. The migration script adds the column as the last position, unless specified with the option after, e.g. 'after' => 'last_time_active'.

You can use cake migrations rollback and cake migrations migrate to re-run the migration, and check how the table looks with desc users; in the mysql shell.

[jiru]

The wording Active this week is not entirely accurate, because it really is Active within the last 7 days. This week could be interpreted as between Monday and today (or Sunday and today depending on which day you start your weeks on).

Maybe I am being too picky and most people will interpret it correctly. Feel free to ignore this comment if you don’t think it’s a real problem.

[franklindyer]

Good point. I think "active within the past week" will be clearer.

[jiru]

I think only two pieces are missing to get this PR ready.

1. Somehow fill the last_contribution field of existing users using data from the contributions table. This could be done as part of your migration script.
2. Add unit tests to make sure this new feature does not inadvertently get broken in the future. If you are not comfortable writing unit tests, I can do it after merging the PR.

[franklindyer]

Okay, I've taken care of the following:

• removed the legacy migration script and tweaked cakephp migration script
• made cakephp migration script populate default last_contribution values from contributions table
• changed last_contribution field to datetime type
• changed use of exists to use of get with try-except block
• replaced title attribute with md-tooltip element
• improved tooltip text and made it localizable

I'm currently struggling with the unit tests but haven't made much progress. I'll keep playing around with them, but if you want to merge and add the unit tests yourself I won't complain. :-)

[franklindyer]

Okay, I was actually able to add a unit test testing this new functionality. In its current state, the test just posts two sentences as contributor and checks that the last_contribution time increases from one sentence to the next. Did you have something more sophisticated than that in mind?