[00:08:31] 6Collaboration-Team-Backlog, 10Flow, 10Collaboration-Community-Engagement, 10Wikimedia-Site-Requests, 13Patch-For-Review: Please set up the Flow extension on the Konkani Wikipedia (gom) - https://phabricator.wikimedia.org/T128359#2128159 (10Catrope) Flow is now the default for newly created talk pages.... [00:09:31] (03PS3) 10Mooeypoo: Initial version of Special:Notifications Javascript page [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277912 (https://phabricator.wikimedia.org/T129176) [00:14:05] (03CR) 10jenkins-bot: [V: 04-1] Initial version of Special:Notifications Javascript page [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277912 (https://phabricator.wikimedia.org/T129176) (owner: 10Mooeypoo) [02:22:53] See you tomorrow. [02:50:21] 6Collaboration-Team-Backlog, 10Notifications: [betalabs] issues with Pasting Flow board specific links - https://phabricator.wikimedia.org/T130172#2128330 (10Etonkovidova) [02:54:11] 6Collaboration-Team-Backlog, 3Collaboration-Team-Current, 10Flow, 10VisualEditor, and 2 others: Don't strip links when pasting HTML into Flow VisualEditor - https://phabricator.wikimedia.org/T129833#2117358 (10Etonkovidova) Checked in beta - all straightforward cases for copying/pasting on Flow boards in V... [04:00:02] 6Collaboration-Team-Backlog, 10Thanks, 10MediaWiki-Special-pages, 10MediaWiki-Watchlist, 7Design: Add a direct thanks link to entries on Special:Watchlist - https://phabricator.wikimedia.org/T129790#2128432 (10MZMcBride) Dupe of {T51541}? [04:00:20] 6Collaboration-Team-Backlog, 10Thanks, 10MediaWiki-Special-pages, 10MediaWiki-Watchlist: Thanks from Special:Watchlist - https://phabricator.wikimedia.org/T129789#2116391 (10MZMcBride) Dupe of {T51541}? [05:35:39] 6Collaboration-Team-Backlog, 10Flow, 10Collaboration-Community-Engagement, 10Wikimedia-Site-Requests, 13Patch-For-Review: Please set up the Flow extension on the Konkani Wikipedia (gom) - https://phabricator.wikimedia.org/T128359#2128511 (10The_Discoverer) Thanks Catrope. I'll be available from 13:30 UT... [06:30:44] 3Collaboration-Team-Current, 6Design-Research-Backlog, 10Notifications: Invite Users to Take Notifications Survey (Using Notifications Panel) - https://phabricator.wikimedia.org/T128937#2128528 (10Pginer-WMF) [06:32:07] 3Collaboration-Team-Current, 6Design-Research-Backlog, 10Notifications: Invite Users to Take Notifications Survey (Using Notifications Panel) - https://phabricator.wikimedia.org/T128937#2090644 (10Pginer-WMF) I added the description at the top of the ticket (combined with some of the info I added before) so... [09:38:05] 6Collaboration-Team-Backlog, 10Collaboration-Community-Engagement, 10Notifications: Explore ways to avoid cross-wiki spam from welcome bots - https://phabricator.wikimedia.org/T129904#2128749 (10Trizek-WMF) Idea of a new feature, to automatically create user talk page has been raised on {T117958}. [09:43:03] (03CR) 10Legoktm: [C: 032] Unconvolute DiscussionParser::getTextSnippet() [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276687 (https://phabricator.wikimedia.org/T129531) (owner: 10Catrope) [09:43:22] (03CR) 10Legoktm: [C: 032] Remove useless getTextSnippet() call in getContentSnippet() [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276692 (owner: 10Catrope) [09:45:11] (03CR) 10Legoktm: [C: 032] Always show a cross-wiki notifications preference, even if the beta feature is disabled [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276683 (https://phabricator.wikimedia.org/T129077) (owner: 10Catrope) [09:54:25] (03Merged) 10jenkins-bot: Unconvolute DiscussionParser::getTextSnippet() [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276687 (https://phabricator.wikimedia.org/T129531) (owner: 10Catrope) [09:56:09] (03Merged) 10jenkins-bot: Remove useless getTextSnippet() call in getContentSnippet() [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276692 (owner: 10Catrope) [10:05:06] (03Merged) 10jenkins-bot: Always show a cross-wiki notifications preference, even if the beta feature is disabled [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276683 (https://phabricator.wikimedia.org/T129077) (owner: 10Catrope) [12:24:46] 3Collaboration-Team-Current, 10MediaWiki-Vagrant, 10Notifications, 13Patch-For-Review: cross-wiki notification on mediawiki-vagrant uses port 8080 for foreign wikis even when accessing the page with https on port 4430 - https://phabricator.wikimedia.org/T126378#2129304 (10SBisson) @Etonkovidova try enablin... [13:06:44] (03PS4) 10Matthias Mullie: Let EchoUserNotificationGateway return accurate notification count [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277242 [13:30:08] (03PS1) 10Matthias Mullie: Fix SQLite update issues because of duplicate indexes [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277990 (https://phabricator.wikimedia.org/T126587) [13:54:33] 6Collaboration-Team-Backlog, 10Notifications, 6Wikisource, 7Crosswiki: Change "Error Access to the remote domain was denied." (echo-api-failure-cross-wiki) message and use errorObj? - https://phabricator.wikimedia.org/T129764#2129553 (10MrStradivarius) >>! In T129764#2115370, @Quiddity wrote: > I believe t... [13:56:08] 6Collaboration-Team-Backlog, 10Notifications, 7Crosswiki: Change "Error Access to the remote domain was denied." (echo-api-failure-cross-wiki) message and use errorObj? - https://phabricator.wikimedia.org/T129764#2129569 (10MrStradivarius) [14:22:22] 6Collaboration-Team-Backlog, 10Notifications, 7Crosswiki: Change "Error Access to the remote domain was denied." (echo-api-failure-cross-wiki) message and use errorObj? - https://phabricator.wikimedia.org/T129764#2129738 (10MrStradivarius) I also agree with @Jay8g that it would be good to do something about... [16:21:54] (03CR) 10Addshore: [C: 032] Fix SQLite update issues because of duplicate indexes [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277990 (https://phabricator.wikimedia.org/T126587) (owner: 10Matthias Mullie) [16:34:34] (03Merged) 10jenkins-bot: Fix SQLite update issues because of duplicate indexes [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277990 (https://phabricator.wikimedia.org/T126587) (owner: 10Matthias Mullie) [16:37:26] 3Collaboration-Team-Current, 10Flow, 13Patch-For-Review: Update.php Flow - Error: 1 index flow_wiki_ref_idx_v2 already exists with SQLITE - https://phabricator.wikimedia.org/T126587#2130186 (10Addshore) 5Open>3Resolved Merged! [16:40:56] 6Collaboration-Team-Backlog, 10Thanks, 10MediaWiki-Special-pages, 10MediaWiki-Watchlist, 7Design: Add a direct thanks link to entries on Special:Watchlist - https://phabricator.wikimedia.org/T129790#2130197 (10Dvorapa) [16:40:58] 6Collaboration-Team-Backlog, 10Thanks, 10MediaWiki-Special-pages, 10MediaWiki-Watchlist: Thanks from Special:Watchlist - https://phabricator.wikimedia.org/T129789#2130198 (10Dvorapa) [16:41:01] 6Collaboration-Team-Backlog, 10Thanks: Add thanks links on Special:Watchlist and Special:Contributions - https://phabricator.wikimedia.org/T51541#2130199 (10Dvorapa) [16:41:38] 6Collaboration-Team-Backlog, 10Thanks, 10MediaWiki-Special-pages, 10MediaWiki-Watchlist, 7Design: Add a direct thanks link to entries on Special:Watchlist - https://phabricator.wikimedia.org/T129790#2116410 (10Dvorapa) @MZMcBride right [16:54:11] (03PS3) 10Mooeypoo: Rename MobileNotificationsWrapper to NotificationsWrapper [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277915 [16:55:13] (03PS8) 10Sbisson: [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) [16:58:47] (03CR) 10jenkins-bot: [V: 04-1] Rename MobileNotificationsWrapper to NotificationsWrapper [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277915 (owner: 10Mooeypoo) [17:03:04] (03CR) 10jenkins-bot: [V: 04-1] [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [17:03:59] (03PS9) 10Sbisson: [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) [17:07:51] (03PS4) 10Mooeypoo: Initial version of Special:Notifications Javascript page [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277912 (https://phabricator.wikimedia.org/T129176) [17:13:18] (03CR) 10jenkins-bot: [V: 04-1] Initial version of Special:Notifications Javascript page [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277912 (https://phabricator.wikimedia.org/T129176) (owner: 10Mooeypoo) [17:20:03] (03CR) 10Mooeypoo: [C: 04-1] "Just a few initial comments. I'm going over the behavior and logical structure now, will follow up." (033 comments) [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [17:26:24] Also, it's on the calendar, but I forgot to remind people. I have a dentist appointment later today, so I'll leave at 11:20 Pacific. [17:27:03] mooeypoo, so you want to use a user preference of type 'api'. [17:28:02] (03CR) 10Sbisson: [WIP] Stop counting notifications objects on the client (033 comments) [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [17:28:15] (03PS10) 10Sbisson: [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) [17:28:17] See https://www.mediawiki.org/wiki/Manual:User_preferences#Hidden_API_preferences for (brief) docs, and https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FGettingStarted.git/master/Hooks.php for an example I did for GettingStarted. [18:17:10] stephanebisson, I'm starting to see why you were so meh about the nesting of the model in Echo. I am still somewhat skeptical that a complete flattening is the answer to it, but I do see some of the issues coming up from the current implementation, both because of the counter thing (the fact you're sending the reference all the way "down" is eh, but seems to be needed) and in terms of the Special:Pagge implementation [18:17:41] We should talk about these things going forward for sure. I think we can solve them while getting the benefits from the nesting but still mitigating the problems that it causes [18:19:10] mooeypoo: do you know everything that fires and execute when you mark a foreign bundle as read? this is driving me crazyyy [18:20:54] hehe, yeah it's really confusing... I agree on that. But I don't know that this is not something we can fix without flattening the entire structure. The problem with flattening is that then we are going against the principle of having the groupWidget (UI) formulate itself according to the groupDM, which is an ooui principle in general; also, we'll be missing a bunch of functionality we're getting for free, especially abstraction. [18:21:06] We should talk about this more, though. I see the issues, and you're right that the events are a mess [18:21:39] There has to be something we can do that gives us the benefits we want without the messy parts. Either way, we should consider code changes now rather than later, as we get into SpecialPage [18:22:44] stephanebisson, do you want to have a meeting about the echo structure like we discussed? Next week after Roan comes back? [18:22:57] Let's sort this out. If we want to change the structure, we should do it soon. [18:22:57] mooeypoo: do you see how the counter breaks free of the complex structure to be a single object with a simple purpose? [18:23:11] there is still madness in calling it though :( [18:23:15] Headed to the dentist. See you later. [18:23:43] stephanebisson, I do, but in "return" you are passing its reference around to the entire structure, which I am not sure I em entirely for. On the flip side, there's not much else you can do, unless you complicate things with even more events [18:24:16] mooeypoo: I could have made it static... ;) [18:24:17] matt_flaschen, good luck [18:24:19] haha [18:25:55] stephanebisson, I also encountered some of those issues when working on the SpecialPage and thinking about next steps. I don't think it will be a bad idea to have different viewmodels for the popup and page inherently, but then we should consider whether the nesting itself is something we want in the inner levels of the data or "just" in the way we create the interface. [18:26:48] stephanebisson, there's definitely a large chunk of the code that's overly complicated right now. [18:27:21] I'm just worried about waiting too long to fix these issues with the code, since it's basically our buiding blocks and we're about to build more complicated stuff on top [18:28:06] mooeypoo: I'm all for making it simpler sooner than later but I'm quite puzzled about the next steps and how to keep it in the ooui spirit [18:28:21] stephanebisson, we might want to decide that the "ooui spirit" should be different. [18:29:06] OOUI is a tool for us. IT doesn't have to work the way we use it now. It might be event driven and we usually use the model to construct the widget, but that's not the way it *must* work [18:29:47] We probably won't go as far as to have a stateless rendering, but we can definitely go away from having a super-complex exaggerated-event-driven nesting structure [18:31:13] mooeypoo: ok, one thing that's confusing me: should events be used to a) update the UI; b) recompute associated state (i.e. update count); c) trigger actions against the API ? [18:31:28] if you say "all of the above" I'll faint [18:32:04] All of the above (just so you faint) [18:32:25] no, hehe, I think that they should be used to update the UI mostly [18:32:26] I'm having a stroke, you're happy? [18:32:49] Did it trigger an event? I wouldn't know otherwise [18:32:58] yes [18:33:42] So, the way i think it probably should happen, is that the UI calls for changes in the model ( onMarkReadButtonClick() { model.markAsRead(); ) etc ) and the model should emit events for the UI to update itself [18:33:52] I think if we clarify the overall Flow (and make it consistent) in the app, we'll be mostly there [18:34:05] but it's also okay imho for the individual parts of the dm to update each other on their state changes. [18:34:22] stephanebisson, yeah I agree, that's why I thik we should sit and hash it out [18:35:15] stephanebisson, how's this: We can have a preliminary meeting next week with whoever wants to join and see if we can hash out at least the direction we all agree on. Then we can either pair or split the work to come up with actionable code structure changes, and then we can implement those. [18:36:16] mooeypoo: yes, I don't much how much we can agree on during a meeting but yes [18:38:35] just fyi, in typical MVC: UI-(triggers actions on)->Controller->(handles the logic and update)->Model->(send events to update)->UI [18:39:37] stephanebisson, yes, that should be the way things happen, except we don't quite hav a controller, and our model is split up to multiple elements, mostly due to OOUI (but also some practical reasons) [18:40:20] yeah, model is always a tree of objects [18:40:47] ... but not always nested.... :P [18:41:11] but going through a controller enforces a unidirectional relationship between any 2 components, and that helps [18:41:41] That's a good point [18:42:01] How would you see a controller in this structure, though? Or are you saying rewrite the entire thing? [18:42:26] (both are acceptable [though may have to be subjected to time restrictions] - I'm just curious to see how we would implement that) [18:42:39] I don't know, hopefully not [18:43:34] time restrictions: we have to consider the cost of doing it against the cost of not doing it [18:43:39] Indeed [18:43:46] I share your worry about the future [18:44:03] that's why I want us to talk this out sooner rather than later in as actionable result as possible [18:44:04] 6Collaboration-Team-Backlog, 10Notifications: [android] Notifications - truncation issues - https://phabricator.wikimedia.org/T130239#2130676 (10Etonkovidova) [18:44:16] anyway, in the very short term, I propose you help me with the counter patch... [18:44:31] because I'm selfish and going crazy [18:45:03] hehe yeah, I'm going over the structure. Only thinkg Im a little iffy about is passing around the reference all the way downwards, but the only alternative I see to this is once again adding events, and I think that may actually push you over the sanity edge. [18:45:09] Also, it's probably not the right thing to do. [18:45:16] But the first reason is more compelling. [18:46:32] my first implementation had the counter behavior as functions on NotificationsModel, and it made me cry [18:48:32] I'm pushing an update that rejects some more setCount estimates... because -4 is probably wrong [18:48:59] (03PS11) 10Sbisson: [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) [18:49:37] the interaction with autoMarkAsRead the first time you open the alert flyout is broken [18:51:31] arn't we getting rid of the auto-mark-as-ready in general? [18:55:32] stephanebisson, I may cause an irreversible twitching for you with this, but it might actually not be a bad idea to have the counter (eventually) as a static variable; we are using it in the popup, but we will likely also need it in the Special Page, and since the special page JS is loaded separately from the badges (that also appear on that page, but are defered in loading) it might be a good idea to share that object... *and* the [18:55:33] Echo API object, coming to think of it. [18:55:50] (03CR) 10jenkins-bot: [V: 04-1] [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [18:58:01] mooeypoo: yeah maybe [18:58:52] (03PS12) 10Sbisson: [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) [18:58:55] (03CR) 10Mooeypoo: [C: 04-1] "Overall, looks good." (032 comments) [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [19:02:31] 6Collaboration-Team-Backlog, 10Notifications: [iOS] Notifications 'Mark as read/unread' not displayed properly - https://phabricator.wikimedia.org/T130244#2130742 (10Etonkovidova) [19:09:48] (03CR) 10Sbisson: "Thanks for reviewing." (032 comments) [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [19:13:18] 6Collaboration-Team-Backlog, 10Notifications: [iOS] Text excerpts display letters with descenders cut off - https://phabricator.wikimedia.org/T130246#2130787 (10Etonkovidova) [20:10:04] 6Collaboration-Team-Backlog, 10Notifications, 7Browser-Support-Apple-Safari: [iOS] Notifications 'Mark as read/unread' not displayed properly - https://phabricator.wikimedia.org/T130244#2131107 (10Legoktm) [20:27:01] (03CR) 10Legoktm: [C: 032] MentionPresentationModel: use Title::equals() [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277683 (owner: 10Catrope) [20:30:58] (03PS2) 10Legoktm: Add i18n/en.json authors [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276949 (owner: 10Amire80) [20:31:01] mooeypoo: in mobile front-end, what is setting the badge count initially, before the Echo stuff is loaded? [20:31:19] (03CR) 10Legoktm: [C: 032] "PS2: Removed duplicate entry for Krenair" [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276949 (owner: 10Amire80) [20:36:13] (03Merged) 10jenkins-bot: MentionPresentationModel: use Title::equals() [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277683 (owner: 10Catrope) [20:42:03] mooeypoo: so mobile front-end shows foreign notifications but doesn't include them in the count... or is it my setup that's broken? [20:42:27] (03Merged) 10jenkins-bot: Add i18n/en.json authors [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276949 (owner: 10Amire80) [21:00:18] stephanebisson, no, you're right [21:00:22] crap [21:00:40] what is it supposed to do? [21:00:55] It's supposed to show the same count as the regular desktop version badge [21:01:05] with the foreign? [21:01:07] but since you're working on the counter, i think it's okay if we want ti implement *that* [21:01:08] yeah [21:01:16] well, with my patch it will [21:01:20] yeah [21:01:27] when it lands, eventually [21:02:47] actually, the server side (or mobile front-end) is also fetching the count the wrong way [21:03:03] but that can be fixed easily in a different patch [21:04:28] Interesting [21:04:33] What wrong way is there? [21:04:37] I thought we'd centralized this stuff [21:04:52] The NotifUser code handles cross-wiki notifs transparently, right? [21:05:35] Krenair: Thanks for doing all of the code review :) [21:05:41] Sorry [21:05:50] legoktm: Thank YOU for +2ing all the things [21:06:07] heh :) [21:06:24] Roan-flying: what was the thing you were complaining that $this->user was protected about? [21:07:50] legoktm: So for "is this the user's talk page" I want to do $this->title->equals( $this->user->getTalkPage() ) [21:07:53] (03CR) 10Mooeypoo: [WIP] Stop counting notifications objects on the client (031 comment) [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) (owner: 10Sbisson) [21:07:58] (In FlowPresentationModel) [21:08:03] But I can't, because $this->user is private [21:08:19] However, $this->getViewingUserForGender() is public and returns the user name, so the workaround is [21:08:39] $this->title->getNamespace() === NS_USER_TALK && $this->title->getText() === $this->getViewingUserForGender() [21:09:02] Which is ugly, but not necessarily ugly enough that it's a slam dunk argument for making $this->user protected instead of private [21:09:53] Roan-flying: MWEchoNotifUser::newFromUser( $user )->getNotificationCount() reads from the current db, or am I missing something? [21:10:29] 3Collaboration-Team-Current, 10Notifications: Cross-wiki notifications bundle should list wikis in order of timestamp - https://phabricator.wikimedia.org/T130298#2131910 (10Catrope) [21:11:32] stephanebisson: Argh, right. I didn't realize that was being used directly, but MF obviously has to [21:11:41] Why is almost every function in that class public :/ [21:12:01] the Echo APi does if ( $foreignNotifications ) { $rawCount += $foreignNotifications->getCount(); } [21:12:20] The desktop code uses getAlertCount() and getMessageCount() which wrap getNotificationCount() [21:12:48] stephanebisson: Ugh that's terrile [21:13:01] And duplicates getAlert/MessageCount()! (At least in the per-section part of that code) [21:13:07] the NotifUser caching model is broken and needs to be rewritetn [21:13:12] there should probably be a higher level facade that encapsulate that and the options (or beta feature enableness) lookup [21:14:06] Yes [21:14:27] Or at least getNotificationCount() should be protected and only the wrappers that add foreign stuff should be public [21:14:54] (Or move the meat of getNotificationCount into a protected helper and make it a wrapper around that, same result) [21:15:06] legoktm: What's broken about the caching model? [21:15:39] is it generally accepted to do internal API calls between subsystems (Echo and mobile front-end) for things like that? [21:15:48] (03PS2) 10Catrope: Sort wikis by timestamp of most recent notification [extensions/Echo] - 10https://gerrit.wikimedia.org/r/273569 (https://phabricator.wikimedia.org/T130298) [21:16:08] stephanebisson: What do you mean by internal API calls in this context? [21:17:00] lunching [21:17:39] Roan-flying: I mean, MF calling the Echo API internally so it depends on the API and not any random class in it [21:17:40] quiddity: FYI https://phabricator.wikimedia.org/T130298 / https://gerrit.wikimedia.org/r/273569 for when you get back [21:17:56] but I'm not sure if it's setup for it technically [21:18:05] mooeypoo: BTW https://gerrit.wikimedia.org/r/273569 is what I was talking about the other day about moving the source remapping thing to the server, please take a look when you have time [21:18:16] Oh you mean the api.php API [21:18:23] We do have a facility for doing internal api.php calls [21:18:31] well, ideally not at the http layer, below it [21:18:32] But it doesn't seem to be the preferred way of doing things [21:18:45] Yes, we have a layer just below the HTTP layer that allows for internal api.php calls [21:19:05] However it appears that people generally prefer the façade approach you mentioned [21:19:23] I think it can be kind of the same thing [21:19:42] Yeah conceptually it's not all that different [21:19:43] as long as it is a known public interface and treated as such [21:19:49] Exactly [21:19:58] And ideally I'd like NotifUser to be that interface [21:20:07] It kind of already is, except that way too many of its methods are public [21:20:26] I think with more wrappers and more hiding/encapsulation it could be a nice interface without too many changes [21:20:34] (03CR) 10jenkins-bot: [V: 04-1] Sort wikis by timestamp of most recent notification [extensions/Echo] - 10https://gerrit.wikimedia.org/r/273569 (https://phabricator.wikimedia.org/T130298) (owner: 10Catrope) [21:20:36] Because I suspect that's sort of what it was intended to be [21:21:31] (03PS3) 10Catrope: Sort wikis by timestamp of most recent notification [extensions/Echo] - 10https://gerrit.wikimedia.org/r/273569 (https://phabricator.wikimedia.org/T130298) [21:24:34] (03CR) 10Catrope: [C: 04-1] "-1 for my COUNT(*) comment from earlier. If Jaime says it's OK (adding him as a reviewer) then I'm fine with it, but I don't want to put a" [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277242 (owner: 10Matthias Mullie) [21:25:18] 6Collaboration-Team-Backlog, 10Notifications: [mobile] History/topic pages do not use mobile interface - Notification panel displayed blank - https://phabricator.wikimedia.org/T130309#2132134 (10Etonkovidova) [21:31:05] (03CR) 10Catrope: [C: 032] Corresponding changes for Echo 99+ refactoring [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276099 (https://phabricator.wikimedia.org/T127288) (owner: 10Mattflaschen) [21:34:04] (03PS13) 10Sbisson: [WIP] Stop counting notifications objects on the client [extensions/Echo] - 10https://gerrit.wikimedia.org/r/277609 (https://phabricator.wikimedia.org/T129726) [21:36:57] (03CR) 10Catrope: [C: 032] Make plural support for large values (100 or more) explicit in l10n [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276096 (https://phabricator.wikimedia.org/T127288) (owner: 10Mattflaschen) [21:39:23] Roan-flying: stores counts in different memcache keys, doesn't have memcache invalidation and lazy load on next request, it just does the master query and updates the cache even if you don't need the new value right now, isn't using WANCache, etc. [21:40:19] legoktm: That sounds like it's related to T106276 [21:40:19] T106276: Certain Memcached keys are retrieved multiple times for a single page view - https://phabricator.wikimedia.org/T106276 [21:40:27] the first part is [21:44:20] 6Collaboration-Team-Backlog, 10Notifications: [mobile] No padding for text in topic titles - https://phabricator.wikimedia.org/T130313#2132234 (10Etonkovidova) [21:46:48] 3Collaboration-Team-Current, 6Design-Research-Backlog, 10Notifications: Invite Users to Take Notifications Survey (Using Notifications Panel) - https://phabricator.wikimedia.org/T128937#2132263 (10jmatazzoni) @Quiddity asks about the following: > We'd also discussed showing the invitation to: "...currently... [21:51:15] (03CR) 10Catrope: [C: 032] Block undeleting an article over an existing Flow board [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277536 (https://phabricator.wikimedia.org/T104591) (owner: 10Matthias Mullie) [21:53:27] mooeypoo, did you see my links about API preferences? [21:53:43] (03Merged) 10jenkins-bot: Make plural support for large values (100 or more) explicit in l10n [extensions/Echo] - 10https://gerrit.wikimedia.org/r/276096 (https://phabricator.wikimedia.org/T127288) (owner: 10Mattflaschen) [21:53:46] (03Merged) 10jenkins-bot: Corresponding changes for Echo 99+ refactoring [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276099 (https://phabricator.wikimedia.org/T127288) (owner: 10Mattflaschen) [21:53:53] matt_flaschen, can you repost? I'm just now looking for that. I'm adding a config var, but not sure how to read/write to it in php [21:54:47] mooeypoo, so you want to use a user preference of type 'api'. [21:54:53] See https://www.mediawiki.org/wiki/Manual:User_preferences#Hidden_API_preferences for (brief) docs, and https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FGettingStarted.git/master/Hooks.php for an example I did for GettingStarted. [21:55:49] mooeypoo, it depends on both user preference and other info (e.g. number of notificiations or something), right? [21:55:59] So, you want to use MakeGlobalVariablesScript to inject a wg to the client side. [21:57:04] You define an API preference as shown in Hooks.php and use $user->getOption to check the current value. When they click X to dismiss on the client-side, use https://www.mediawiki.org/wiki/API:Options to change the preference. [21:59:36] mooeypoo, I think that should cover it. There might be another approach (user.options) if all you need on the client-side is the user preference itself (user.options), whether the feature is enabled (wg from ResourceLoaderGetConfigVars), and information already available on the client-side (e.g. current notification count). [22:04:19] (03CR) 10jenkins-bot: [V: 04-1] Block undeleting an article over an existing Flow board [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277536 (https://phabricator.wikimedia.org/T104591) (owner: 10Matthias Mullie) [22:08:44] matt_flaschen, thanks, reading now [22:10:19] (03CR) 10Catrope: [C: 032] "Bogus failure: 21:51:59 Error: Cannot find module '/usr/local/bin/npm'" [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277536 (https://phabricator.wikimedia.org/T104591) (owner: 10Matthias Mullie) [22:11:35] 6Collaboration-Team-Backlog, 10Notifications: [mobile] Cannot add a new topic on Flow pages - https://phabricator.wikimedia.org/T130319#2132379 (10Etonkovidova) [22:14:34] By "whether the feature is enabled" I mean wiki-level, not user-level. [22:17:21] matt_flaschen, yep. I am using a global variable $wgEchoShowFooterNotice to enable per wiki, and then [22:17:22] $preferences['echo-dismiss-feedback-alert'] = array( [22:17:22] 'type' => 'api', [22:17:22] ); [22:17:35] ^^ to define the variable itself [22:18:10] mooeypoo, what are the other criteria for whether it's shown? [22:18:11] 6Collaboration-Team-Backlog, 10Notifications: [mobile] Edit pencil icon displayed as protected unnecessarily - https://phabricator.wikimedia.org/T130322#2132457 (10Etonkovidova) [22:18:23] matt_flaschen, minimum 5 unread notifications, but I do that in the UI [22:19:44] mooeypoo, okay, so you want ResourceLoaderGetConfigVars and mw.user.options.get, and don't need the MakeGlobalVariablesScript, but you probably already got most of that. [22:20:08] matt_flaschen, I'm doing: [22:20:09] $sk->getOutput()->addJsConfigVars( 'wgEchoDismissFeedbackAlert', ( [22:20:09] $wgEchoShowFooterNotice && [22:20:10] $user->getOption( 'echo-dismiss-feedback-alert' ) [22:20:10] ) ); [22:20:31] That should give the proper boolean value [22:21:42] (03Merged) 10jenkins-bot: Block undeleting an article over an existing Flow board [extensions/Flow] - 10https://gerrit.wikimedia.org/r/277536 (https://phabricator.wikimedia.org/T104591) (owner: 10Matthias Mullie) [22:28:42] mooeypoo, it will be more efficient if you don't use addJsConfigVars/ResourceLoaderGetConfigVars. [22:28:56] mooeypoo, you already have the mw.user.options.get on the client-side. [22:29:19] (03PS3) 10Catrope: Use strong and em instead of wiki markup bold and italics [extensions/Echo] - 10https://gerrit.wikimedia.org/r/274660 (owner: 10Siebrand) [22:29:26] mooeypoo, wait, one sec. [22:29:43] But then how does she take wgEchoShowFooterNotice into account? [22:30:18] (03CR) 10Catrope: [C: 032] Use strong and em instead of wiki markup bold and italics [extensions/Echo] - 10https://gerrit.wikimedia.org/r/274660 (owner: 10Siebrand) [22:31:40] Roan-flying, yeah, I got confused for a second. addJsConfigVars is equivalent to MakeGlobalVariablesScript, not ResourceLoaderGetConfigVars [22:31:52] mooeypoo, you need user.options + "one more thing". [22:32:01] But if the "one more thing" is ResourceLoaderGetConfigVars, it's better cached. [22:32:27] Whereas your addJsConfigVars is partially duplicative (since you already have user.options) and included in every page's HTML. [22:32:39] ResourceLoaderGetConfigVars is cached in the startup module. [22:32:45] (03PS2) 10Catrope: Use strong and em instead of wiki markup bold and italics [extensions/Flow] - 10https://gerrit.wikimedia.org/r/274659 (owner: 10Siebrand) [22:32:54] Oh, yes [22:33:00] You should prefer getConfigVars where possible [22:33:24] And separating the config var and the user preference lets you do that, because getConfigVars can't look at the user [22:37:53] (03CR) 10jenkins-bot: [V: 04-1] Use strong and em instead of wiki markup bold and italics [extensions/Flow] - 10https://gerrit.wikimedia.org/r/274659 (owner: 10Siebrand) [22:40:21] (03PS3) 10Catrope: Use strong and em instead of wiki markup bold and italics [extensions/Flow] - 10https://gerrit.wikimedia.org/r/274659 (owner: 10Siebrand) [22:42:08] (03PS3) 10Catrope: Notify when a topic is marked as resolved or reopened [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276480 (https://phabricator.wikimedia.org/T125654) (owner: 10Matthias Mullie) [22:42:43] (03CR) 10jenkins-bot: [V: 04-1] Notify when a topic is marked as resolved or reopened [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276480 (https://phabricator.wikimedia.org/T125654) (owner: 10Matthias Mullie) [22:43:14] (03CR) 10jenkins-bot: [V: 04-1] Use strong and em instead of wiki markup bold and italics [extensions/Echo] - 10https://gerrit.wikimedia.org/r/274660 (owner: 10Siebrand) [22:46:34] eh I'm confused [22:46:54] mooeypoo, okay, so there are two sources of mw.config.get: [22:46:58] matt_flaschen, if I use $vars['wgEchoDismissFeedbackAlert'] = ... I need more than "just" the $wgEchoShowFooterNotice variable, I also need the user config [22:47:06] in onResourceLoaderGetConfigVars [22:47:08] 1. startup module; heavily cached. [22:47:16] 2. JS embedded in HTML. [22:47:49] mooeypoo, you want to use #1 + mw.user.options.get( 'echo-dismiss-feedback-alert' ) [22:48:10] that's in js [22:48:17] Yes [22:48:19] right [22:48:58] wait, mw.user.options.get or mw.config.get ? [22:49:05] Both. [22:49:19] mw.config.get for wgEchoDismissFeedbackAlert and mw.user.options.get for echo-dismiss-feedback-alert [22:49:32] oh [22:49:34] wait [22:49:36] okay, hang on [22:49:43] We can pair if you want. [22:50:31] yeah I'm really confused [22:51:57] wgEchoDismissFeedbackAlert is what I called the echo-dismiss-feedback-alert in addJsConfigVars [22:52:02] ... matt_flaschen you p for a quick hangout? [23:02:11] (03PS4) 10Catrope: Notify when a topic is marked as resolved or reopened [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276480 (https://phabricator.wikimedia.org/T125654) (owner: 10Matthias Mullie) [23:02:28] (03CR) 10jenkins-bot: [V: 04-1] Notify when a topic is marked as resolved or reopened [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276480 (https://phabricator.wikimedia.org/T125654) (owner: 10Matthias Mullie) [23:03:16] (03CR) 10Catrope: [C: 04-1] Notify when a topic is marked as resolved or reopened (033 comments) [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276480 (https://phabricator.wikimedia.org/T125654) (owner: 10Matthias Mullie) [23:06:44] (03CR) 10Catrope: Notify when a topic is marked as resolved or reopened (031 comment) [extensions/Flow] - 10https://gerrit.wikimedia.org/r/276480 (https://phabricator.wikimedia.org/T125654) (owner: 10Matthias Mullie) [23:11:12] 6Collaboration-Team-Backlog, 10Flow, 10Collaboration-Community-Engagement, 10Wikimedia-Site-Requests, 13Patch-For-Review: Please set up the Flow extension on the Konkani Wikipedia (gom) - https://phabricator.wikimedia.org/T128359#2132889 (10Catrope) Sorry, I ended up being busy and not seeing your messag... [23:29:36] 6Collaboration-Team-Backlog, 10Thanks, 7JavaScript: Non-breaking spaces break page layout when confirming thanks in diff view - https://phabricator.wikimedia.org/T111735#2132927 (10darthbhyrava) a:3darthbhyrava I'll take this up, and go ahead. :) [23:37:25] matt_flaschen, Im getting a really weird error from the API when I try to update the user preference: "Validation error for 'echo-dismiss-feedback-alert': not a valid preference" [23:37:28] O.o [23:38:06] mooeypoo, are you sure it's properly registered? Hook in hook class, hook registered? [23:40:06] looks it :\ I'm using existing hooks [23:40:17] ok I got it [23:40:46] when I registered, I conditionally registered based on whether $wgEchoShowFooterNotice is true [23:40:52] ... but I forgot to add global $wgEchoShowFooterNotice [23:43:28] mooeypoo, see my PM. [23:46:10] mrrrrrrrrrrr now I'm getting the value as "true" rather than true [23:46:13] * mooeypoo grrs [23:46:47] this is ridiculous [23:47:12] matt_flaschen, the default value i set up is a boolean. So !mw.user.options.get( 'echo-dismiss-feedback-alert' ) works. But then when I update through the API, the value becomes a string (!!!!) and now !mw.user.options.get( 'echo-dismiss-feedback-alert' ) doesn't work. [23:47:18] wtf. Am I updating it wrong or something? [23:47:45] Accio MatmaRex [23:47:55] * MatmaRex appears [23:48:04] mooeypoo, no, I think this is a Known Lameness of the API. [23:48:06] WOW you guys. [23:48:16] I think MatmaRex has been plotting to fix this. [23:48:17] oh yeah, it's fucked, but that's notmal [23:48:17] IIRC [23:48:30] That was both epic and omg'ish at the same time. Well done. [23:48:42] mooeypoo: you need to handle both falsy values and '0'. [23:48:47] (if you want to check for false) [23:48:50] ok, so, how do I deal with this? Is it bad if I change to 1 and 0--- oh [23:49:16] MatmaRex, is there something that will cause it to go to '0' even if default is false? [23:49:43] so, the only thing you can store in preferences are strings [23:49:57] but we don't store the default values [23:50:03] and we don't correctly cast them to strings [23:50:19] Right, so if we only store '1', and the default is false, it should only come out as '1' or false, in theory right? [23:50:28] Still, might be safer to check for '0'. [23:50:32] so if you set the default to false or 0, you get that back in mw.user.options [23:51:13] Safer to check for '0' in addition I mean to false I mean. [23:51:21] yeah [23:51:47] matt_flaschen: i'm not sure how it compares the values when saving. it might store '0' when the default is 0 or false. it definitely won't store it if the default is '0'. [23:52:53] meh. okay, default = 0, and if i set it, it's 1, and I check for !value so 0 catches and 1, whther it's a number or a string, doesn't [23:52:59] Don't bother storing default values [23:52:59] $defaultOption = self::getDefaultOption( $key ); [23:52:59] if ( ( $defaultOption === null && $value !== false && $value !== null ) [23:52:59] || $value != $defaultOption [23:52:59] ) { [23:53:30] so… it uses whatever exact semantics the PHP loose comparison == has. which i really don't feel like looking up [23:53:49] MatmaRex, we're only reading and writing it on the client-side, and should only ever write 1/true. Default is false. [23:54:05] ok I solved it, guys, it works [23:54:11] 0 for default, 1 for set [23:54:13] matt_flaschen: well, no idea then [23:54:18] it's stupid, but it works [23:54:22] there's a bug somewhere [23:54:26] ;) [23:55:16] mooeypoo, per MatmaRex, you may want to also check for '0' (which is truthy in JS). Not sure if really necessary. [23:55:34] matt_flaschen, it doesn't seem to ever happen, though, since I don't set 0 ever [23:55:40] Yeah [23:56:40] 6Collaboration-Team-Backlog, 6Editing-Department, 10Reading-Web, 7JavaScript, and 2 others: Add user settings frontend JavaScript API - https://phabricator.wikimedia.org/T96155#2132991 (10ori)