Skip to content

Conversation

m90
Copy link
Contributor

@m90 m90 commented Jun 17, 2024

Tickets https://phabricator.wikimedia.org/T366702 and https://phabricator.wikimedia.org/T364991

Previously, the logic was built on the assumption that the first revision to consider:

  • always has the id 1
  • always exists

This created two problems:

  • sometimes id 1 did not exist
  • since introducing the prefilled main page, the first revision would always be created instantly on behalf of the platform user

Instead, this PR changes the logic to be:

  • get the id of the oldest revision that has not been created by the PlatformReservedUser
  • if found, fetch the timestamp for that revision in a second call

which fixes both problems

@m90 m90 force-pushed the fr/stats-fixes branch 2 times, most recently from 427c597 to 28d89bc Compare June 17, 2024 10:52
Currently two things go wrong:
- revid 1 does not neccessarily exist
- the platform reserved user should be excluded
@m90 m90 force-pushed the fr/stats-fixes branch from 28d89bc to e26f98f Compare June 17, 2024 11:07
@m90 m90 force-pushed the fr/stats-fixes branch from 0b7e28a to b2b3c7b Compare June 18, 2024 07:37
'format' => 'json',
'prop' => 'revisions',
'rvprop' => 'timestamp',
'revids' => $firstRevision,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to use 'formatversion' => 2? Currently, the API returns a map of pages instead of an array, which breaks the subsequent data_get(..., 'query.pages.0.revisions.0.timestamp').

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, good catch, that got lost when I refactored things lateron. WIll fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this.

@m90 m90 merged commit ffc0949 into main Jun 18, 2024
@m90 m90 deleted the fr/stats-fixes branch June 18, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants