Skip to content

Dynamically retrieve list of supported languages#46

Merged
samwilson merged 1 commit intomasterfrom
lang-list
Dec 8, 2016
Merged

Dynamically retrieve list of supported languages#46
samwilson merged 1 commit intomasterfrom
lang-list

Conversation

@samwilson
Copy link
Member

The list of languages supported by CopyPatrol is retrieved from
the database, and cached for 7 days. The dropdown list in the
navbar now lists all these.

The phpcs change is just to ignore the cache directory that
exists for local development.

Bug: https://phabricator.wikimedia.org/T152362

$langs = $this->getPlagiabotDao()->getLanguages();
$cacheItem->expiresAfter( new DateInterval( 'P7D' ) );
$cacheItem->set( $langs );
$this->slim->cache->save( $cacheItem );
Copy link
Member

@MusikAnimal MusikAnimal Dec 6, 2016

Choose a reason for hiding this comment

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

Can't remember, where does the Slim cache live? In a cookie?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, Redis when in production, or a local file-based cache when in development (or any time we don't have REDIS_HOST defined).


// Otherwise get the list from the database, and cache it for a week.
$langs = $this->getPlagiabotDao()->getLanguages();
$cacheItem->expiresAfter( new DateInterval( 'P7D' ) );
Copy link
Member

Choose a reason for hiding this comment

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

maybe also explain what P7D is. I'm safely guessing 7D = 7 days, but what does the P do?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the interval_spec, quite redundant really, I guess their aiming at future compatibility with things. I think it's also something to do with ISO8601 (with the 'T' being a delimiter for the time component)....

The format starts with the letter P, for "period." Each duration period is represented by an integer value followed by a period designator. If the duration contains time elements, that portion of the specification is preceded by the letter T.

I'll make a comment about the format meaning.

'app' => $this->slim,
'i18nCtx' => $this->slim->i18nContext
'i18nCtx' => $this->slim->i18nContext,
'supportedLanguages' => $this->getSupportedLanguages(),
Copy link
Member

Choose a reason for hiding this comment

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

does the trailing comma violate any coding conventions (not sure)? I know some organizations prefer it this way

Copy link
Member Author

Choose a reason for hiding this comment

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

The mediawiki-codesniffer master doesn't complain about it, and I like doing trailing commas in multi-line arrays becuase it means the next time something gets added the diff then shows an actual meaningful change of 1 line.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I like it too :) As long as the codesniffer is happy, you don't want to upset it

Copy link
Member Author

Choose a reason for hiding this comment

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

Talking of codesniffer, do you fancy reviewing this? https://gerrit.wikimedia.org/r/#/c/324376/ :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't have any coding conventions against trailing commas in PHP, in fact, this is probably the more common style in MediaWiki as it helps to prevent accidental syntax errors when adding new items to arrays. Of course, you have to be careful not to do this in JavaScript though.

@samwilson samwilson force-pushed the lang-list branch 2 times, most recently from 41899fc to 267beaa Compare December 7, 2016 08:39
@samwilson
Copy link
Member Author

I've added messages for all languages supported by Turnitin (for the future).

The list of languages supported by CopyPatrol is retrieved from
the database, and cached for 7 days. The dropdown list in the
navbar now lists all these. Translations for all Wikipedias'
titles have been added.

The phpcs change is just to ignore the cache directory that
exists for local development.

Bug: https://phabricator.wikimedia.org/T152362
Copy link
Collaborator

@kaldari kaldari left a comment

Choose a reason for hiding this comment

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

+1, looks good to me, but haven't tested.

@samwilson samwilson merged commit 2d14179 into master Dec 8, 2016
@samwilson samwilson deleted the lang-list branch December 8, 2016 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants