-
-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hook to pass certain extra field data from cw_requests to cw_wikis #579
base: master
Are you sure you want to change the base?
Conversation
Check commit and GitHub actions for more details
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
} | ||
|
||
public function setExtraFieldData( string $field, mixed $value ): void { | ||
if ( $value !== $this->getExtraFieldData( $field ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically I like early returns but decided not to here as my plan is to eventually make RemoteWikiFactory more similar to how WikiRequestManager works which means it being more inline with that here makes it a tiny bit easier down the line which is why I decided against it. But I guess it wouldn't be horrible to early return also...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright then, feel free to resolve this conversation if you decide not to early return
|
||
$this->extra = $extra; | ||
$this->trackChange( $field, $this->getExtraFieldData( $field ), $value ); | ||
$this->newRows['wiki_extra'] = $newExtra; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal Note: This definitely needs tested with more than one field at once (calling this function more than once) as unlike RequestWiki we can't have just a set all field data at once function here, so like the first version of WikiRequestManager this may have an issue with that and will need to be tested and handled some other way if so.
In the future, this will make us be able to remove the
addNewRow()
method from RemoteWikiFactory and be able to migrate things we use in mw-config or MirahezeMagic likeprimary-domain
andmediawiki-version
from our own custom table fields we added to thecw_wikis
table to be natively possible in CreateWiki using the hook and the natively-availablewiki_extra
field.It will also allow us to remove the fake variable in WikiDiscover,
$wgWikiDiscoverDescription
to actually make it not have to just read fake variable from ManageWikSettings and instead be possible by just pulling the description from thiswiki_extra
field.The other benefit this will bring will also be allowing to select the primary domain (which we use a custom field and hook in ManageWiki for) directly from RequestWiki without having to rely on our custom fields or an additional custom field in
cw_requests
, so this will allow us to bring selecting the primary domain directly from RequestWiki as well.This has many more benefits for external use as well, making CreateWiki much more expandable utilizing hooks rather than custom very hacky overrides.