[05:11:17] Glaisher: any chance you are around ? [05:11:29] here [05:11:50] Glaisher: great. just wanted to confirm that you meant Special:Newsletter/55/manage?oldid=3 to point to the edit screen ? [05:12:25] uhm, maybe you are meaning something else [05:13:02] Special:Newsletter/manage?oldid=54561 and Special:Newsletter/manage won't have any difference [05:13:04] that's what I meant [05:13:57] Maybe we should kill the Special:Newsletter/manage page entirely and just provide the form on the page itself [05:14:21] redirecting to a completely new page probably does weird things [05:17:09] Glaisher: 'and just provide the form on the page itself' --> like when someone clicks on 'Mangae', he gets an editable version in Newsletter:NewsletterName itself ? [05:17:40] yeah, action=edit's content would look like the manage form [05:18:00] in which case, we should probably kill Special:CreateNewsletter too [05:18:08] hmm.. this patch is already becoming too huge [05:18:12] and it's hard to review [05:18:12] oh :o [05:18:23] lets keep it in a subsequent patch, then ? [05:19:06] Let's try to do this step by step in smaller commits.. though doing that would probably leave the extension broken sometimes [05:19:23] true that. [05:22:46] tonythomas: I think I got disconnected.. maybe I missed something? [05:23:07] Glaisher: nothing serious. I was just looking at how giving old id affects other edit pages [05:23:13] havent seen this before, that is why! [05:23:22] oh.. [05:23:40] If you provide oldid param, you can view older revisions of the page [05:23:56] for eg. https://meta.wikimedia.org/w/index.php?title=Request_for_an_account_on_the_Foundation_wiki&oldid=15832531 [05:41:24] Glaisher: Looks like I will have to use a hook to find out if the user wants to edit an old rev, and then pass it to the form in manage :o [05:42:34] I don't think you need a hook in the current manage form. Just check for the existence of oldid param, check whether it is valid, and then show the form using data from that revision [05:43:07] but still, I think we need to consider splitting up the patch into several separate patches. What do you think? [05:43:09] right. Let me try doing that [05:43:23] okey. so this one goes to a new patchset ? [05:43:29] I can get that done. [05:43:39] but splitting the current one would be a horror :o [05:43:57] Ok, I think we need to a plan first. :) [05:44:19] Switching to ContentHandler is a huge task. [05:44:34] and probably consists of doing multiple things which we haven't thought of yet [05:45:08] True that, and I think we will need all the basic minimum in one patch [05:45:12] I can also help with working on the code [05:45:17] great! [05:45:55] Ok, let's make a plan and split up the workload then [05:46:37] I haven't worked on ContentHandler related code previously [05:46:38] okey. so - this would involve splitting up the current PS too, right ? [05:46:46] (even I havent :P ) [05:46:59] yeah [05:47:40] Glaisher: everythin in content/ except the custom Diff engine would be required for the contenthandler to work bare [05:49:23] Ok, so first let's work on the code for "create a newsletter" workflow [05:50:31] should we write it somewhere, say in some etherpad or something ? [05:50:43] yeah great idea [05:50:59] http://etherpad.wikimedia.com/p/newsletters-contenthandler [05:51:23] err. https://etherpad.wikimedia.org/p/newsletters-contenthandler [05:51:44] ok, I'm there [05:52:14] me too [06:27:34] tonythomas: Are you working on a new patch or on the existing one? [06:27:57] Glaisher: I can push that eitherway. Do you want me to push it as a separate one above the current one ? [06:28:10] its like only 10 lines more :D [06:28:41] Well, doing that wouldn't make the already huge patch any smaller.. :( [06:29:02] exactly :( [06:29:06] Which object are you instantiating on the hook? [06:29:58] I am not instantiating anything at this point, as the page havent been created yet. Just $query = "newsletter=" . $article->getTitle()->getText(); $title = SpecialPage::getTitleFor( 'CreateNewsletter' )->getFullURL($query); and redirecting there safely [06:30:16] in the form, just taking the value from the request, if exists, and showing it over there. [06:30:36] Hmm, so let's start from there then. For now, nothing to do with ContentHandler [06:30:42] and we can build on from there [06:30:52] I would suggest keeping the exsiting patch as it is [06:30:55] existing* [06:31:03] well for that one we would need ContentHandler for even that one to work :o [06:31:12] there is an if ( $article->getContentModel() == 'NewsletterContent' ) above [06:31:37] Just check for namespace now and add a @todo [06:32:06] This will help with making the code less scary and more easy for me to review and work on too :) [06:32:31] okey. me or you ? [06:32:38] ? [06:32:52] preparing the new patch! [06:33:13] You have already written the code for CustomEdit thing :P [06:33:38] I meant that we can both work on it once we get something started [06:33:40] okey. but its all over the contenthandler patch. I will stash everything, rebase to master, and produce a fresh one. [06:34:06] Yeah, upload a new changeset not over the current one [06:34:18] That would help with copying over the code from that one [06:34:21] okey. on it. [06:39:13] tonythomas: I have to leave now. cc me to it, please and I will look at it later [06:39:21] alright. [06:46:30] Glaisher: added. you are CC'ed [08:20:32] tonythomas: You wouldn't mind even if I edited on the patch, right? :) [08:21:20] Hmm, no namespace setup or anything like that? [08:21:50] Well, I was hoping that we will be starting with this so all the basic stuff should be done in this [08:34:11] Glaisher: oh. I forgot about that one :D [08:34:15] Thank you! [08:34:35] I'll try to work on it for now since I have some time [08:35:05] great. [09:36:10] tonythomas: Updated the patch with a new edit page [09:36:14] instead of redirect [09:36:23] Glaisher: Great [09:36:29] eager to test it out! [09:36:57] It's not final yet; I added some comments for what to do in future [09:37:27] For now, I didn't implement anything related to ContentHandler [09:39:15] Glaisher: okey. so we merge everything later, or go on merging one after the other ? [09:39:22] After the UI part of edit page is finished, we can do the actual ContentHandler related part [09:39:44] tonythomas: Even if not merged, we can do separate patches with one another as dependencies [09:39:54] We splitted them for easy reviewing, remember? :) [09:40:01] true. let it be like that. exactly! [09:40:10] testing that one now! [09:40:30] It won't be that different from what you saw at Special:CreateNewsletter for now [09:40:39] I just mostly copied code from there [09:43:05] looks neat! [09:43:15] we can introduce ContentHandler in the next patch ? [09:44:15] let's finish this before moving onto that [09:44:26] It doesn't even work correctly yet [09:44:31] submissions would fail [09:45:50] yup. I get this feeling that we will have to introduce the contentmodel here to make it submit properly! [09:46:29] as the newsletter currently do not have any specific model for itself! [09:46:47] maybe we can make this EditPage to show up forms and etc [09:48:18] The submission is failing probably due to something going wrong with the web request [09:48:44] It won't create the actual page yet [09:53:06] I am not sure if we should spend time fixing it around, or just convert it to use the Special:CreateNewlsetter format, and push the contents to a Newsletter object (which means to contenthandler) [09:53:53] is the failuer due to defaultContetnModel not set for the namespace in extension.json ? [09:54:15] I removed that one line while slicing the original change [09:55:31] It's failing because the form's action attribute isn't set properly and something goes wrong with the request. [09:55:54] You mean, you want to do the redirect from Special:CreateNewsletter? [09:57:57] Uploaded a new patch which fixes that issue [09:58:22] Glaisher: just to confirm, we are expecting the Special:CreateNewsletter to show up in index.php/title=Newsletter:FooBar?action=edit right ? [09:58:47] It won't redirect now but the UI would look like it [09:59:03] If you are testing it locally, can you download the patch which I just uploaded now? [09:59:10] The submission issue has been fixed now. [09:59:15] pulled now. testing [10:01:00] Glaisher: for some reason title=Newsletter:FooBar?action=edit is still showing up the same old wikitext editor for me :\ [10:02:21] That url's query params are wrong [10:02:36] index.php?title=Newsletter:FooBar?action=edit [10:03:02] ah. index.php?title=Newsletter:FooBar&action=edit silly me [10:03:04] Thanks! [10:03:11] np :P [10:03:30] Great. I'm there! [10:04:18] Once, we finish this edit page for newsletter creations, we can then actually introduce ContentHandler. [10:04:31] I can see it works fine now! [10:04:33] Currently, it just does what Special:CreateNewsletter did [10:06:18] tonythomas: I have to go now, bye [10:06:38] Glaisher: ciao!