Page MenuHomePhabricator

Test phabricator sprint extension updates
Closed, ResolvedPublic2 Estimated Story Points

Description

Updates to the sprint extension are being staged for release to our production instance of Phabricator, with a release date targeted for Apr 22, 201.

Release Engineering has requested that we test out the updates ASAP to ensure there are no major issues - or surprises - prior to the release date.

The updates can be tested at:
https://phab-01.wmflabs.org/

Related Objects

StatusSubtypeAssignedTask
DuplicateAklapper
ResolvedKrenair
Declinedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
Resolvedmmodell
ResolvedNemo_bis
Resolvedmmodell
ResolvedKLans_WMF

Event Timeline

Awjrichards raised the priority of this task from to High.
Awjrichards updated the task description. (Show Details)
Awjrichards added a project: Team-Practices.
Awjrichards moved this task to To Triage on the Team-Practices board.
Awjrichards added a subscriber: Awjrichards.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 8 2015, 8:07 PM

@Christopher can you provide us with a list of changes?

If we can get test cases written for this, that would be swell, but let's not block getting the functionality tested on having documented test cases.

Awjrichards set Security to None.

The tagged Phabricator revision for the 22 April update according to @chasemp is https://secure.phabricator.com/D12304 (8 April). Sprint needs to be updated to match this revision. I will do this today and tag a new release in the production branch of Sprint for testing.

Christopher added a comment.EditedApr 9 2015, 2:34 PM

This is ready for QA testing. Already deployed to http://phab08.wmflabs.org Please use production branch tag release/2015-04-22 of Sprint.

There are two new "features".

  1. T89275 Implement serialization for Sprint Data [refers to T152 Install PHPExcel so I can export reports] This basically adds a CSV export function to the data tables for tasks and events in a Sprint project. Note: This requires a static SWF file in /webroot. In order for this to work, the files in $sprint/rsrc/webroot-static should be copied to $phabricator/webroot/rsrc/libext. If the SWF file is not there, the feature will not work, but nothing else will break. (@chasemp please provide some guidance on how this file copying issue can be handled in puppet or otherwise)
  1. A form for Sprint creation. This is a rather simple solution to the issues people have with the required start and end dates for a sprint project. It also sets the default icon and color for Sprint projects as requested in T91529. The form is not that friendly, but it works. To request the form, there is the button in the upper right corner of the main Sprint List application view or also in any Burndown View in the same place. The conduit API method that is implemented "sprint.create" will also potentially help with the Phragile application.

See: T95079 Add conduit methods for Sprint Creation

Bug Fixes:
T90661 Sprint extension may be causing weirdness with task views (open vs. all)
T91213 BurnUp Chart does not load on phabricator.wikimedia.org (unfiltered data stream to the D3 JS too large)
T89876 Long task names do not wrap in DataTable column for Task table
T91042 The icons for Sprint Board and Burndown view don't display an active blue on gray state
T94203 Pie charts for points allocation is half off the screen, with no scrolling (this issue is not fixed, but horizontal scrolling has been partially implemented where possible)

Also it includes internal changes to make it current with upstream:
This fixes
T95191 Fatal error when clicking on tag which doesn't yet have workboard created

ksmith added a subscriber: ksmith.

Assigning to Kristen, per discussions in the TPG weekly meeting where we concluded that she is uniquely qualified to do this specific testing.

Assigning to Kristen, per discussions in the TPG weekly meeting where we concluded that she is uniquely qualified to do this specific testing.

any luck here? FYI we will plan on the 22nd but without sign off will delay. :)

@chasemp I had intended to get to this sooner, but I was doing lots of travel and training last week. I will put this at the top of my todo list today and take a spin through the latest sprint extension changes on phab-08 and update this issue with any findings.

@KLans_WMF: If you need help testing I would be glad to help out. Better yet, if you can show me how it's being used (at some time in the future, when you have the time.) I would like to get a better idea of exactly how it's used so that I can test it more effectively. This would also reduce our dependence on you next time we update phabricator or the sprint extension.

KLans_WMF added a comment.EditedApr 20 2015, 11:03 PM

I am testing this as a sprint extension "power user" (I generally create a minimum of 2 sprints/week), and I am also representing other users who use sprints for project management.

Below are issues with new features (1, 2) + bugs (3) that I found during testing on https://phab08.wmflabs.org:

  1. Implement serialization for Sprint Data
    • export to cvs seems to work as one might expect. I suspect that including the Epoch date format as well as the human readable dates may be overkill/confusing to most users, and I would suggest removing the Epoch columns (unless this was a feature request)
  1. A form for Sprint creation.
    • I would strongly recommend against releasing this as it is currently implemented for the following reasons:
      • the core problem around setting sprint start and end dates is that the option to set a sprint start and end dates is not included in the Create Project screen. Users then create projects and have no idea that they need to go back in to the main project page, select 'Edit Details', and then set a sprint start and end date. I don't see how this implementation solves that problem, and I don't see any benefit to adding a Create Sprint step in places that are not intuitive (e.g. the sprint list or the burndown page rather than the project creation step). I recognize that you are addressing the problem of having to automatically set the color and icon for the project type "Sprint", but again, having that solution implemented in a random corner of Phab and not in the Create Project screen is not solving the problem.
      • on the /conduit/method/sprint.create/ screen, I see the error ERR-CONDUIT-CORE: See error message for details.
      • in my view, it is not reasonable to expect the average user to use JSON as a form field input
      • Not sure why the button that is usually "Submit" is labelled "Call Method" in this case (for example, on the create task screen, the button doesn't say createtask)
      • In general on the new sprint creation form, it seems like there's too much of the API logic peeking through in to the UI (see attachment)
      • In summary, this seems like a good start towards pulling the pieces together for streamlining the sprint creation workflow, but it's not yet ready for prime time.
  1. Bug fixes
    • T90661 Sprint extension may be causing weirdness with task views (open vs. all): I can't actually parse what the desired behavior is that this fix addresses (I think that is it for both sprint and on-sprtin boards to default to all tasks?). Behavior when testing: non-sprint board default is open tasks (https://phab08.wmflabs.org/tag/arthurs'_megamanager_report/board/) and sprint board default is all tasks (https://phab08.wmflabs.org/tag/§arthur-sprint-2/board/)
    • T91213 BurnUp Chart does not load on phabricator.wikimedia.org (unfiltered data stream to the D3 JS too large): not sure how to test this...is there a burn chart option?
    • T89876 Long task names do not wrap in DataTable column for Task table: not enough information to test this
    • T91042 The icons for Sprint Board and Burndown view don't display an active blue on gray state: Looks good!
    • T94203 Pie charts for points allocation is half off the screen, with no scrolling (this issue is not fixed, but horizontal scrolling has been partially implemented where possible): as noted, this issue is not fixed, so I would not call this issue fixed, per se (see screenshot attached)
    • T95191 Fatal error when clicking on tag which doesn't yet have workboard created: confirmed fixed
  1. Other:

Did not get in to any detailed regression testing, but clicking around doing the usual stuff seemed fine.

@mmodell that would be great, feel free pop something in my calendar!

@KLans_WMF your response to this task, above, is actually a good start for me understanding how sprint is used. Your testing was very thorough, I'm impressed.

regarding T90661 - the intended behavior is that non-sprint boards show open tasks by default (as they always did before sprint extension broke that default behavior) so it sounds like that bug is fixed, as it should be based on my reading of the code change.

@KLans_WMF thank you for the review. My comments:

  1. Regarding the Sprint creation form. I agree that the form is less than friendly, but this is a Phabricator standard form for the Conduit API. The "Errors" notice message is not actually an error and appears on every conduit method by default. To recreate a new conduit form is problematic for a variety of reasons. The sprint.create API method is an enhancement, so I recommend that the code stay in. Since the "Create Sprint" button is in a "random corner", its usage should only be for people who know how to use it, so I do not think that there is any disadvantage to leaving it as an option. Furthermore, to modify the default Project Creation form (which is your preference) is impossible without forking the controller and re-implementing a new version in the Sprint extension, which is a large maintenance burden that I am not willing to take on at this point. So, as cliche as it sounds, this ugly form is the "best that can be done" within the current constraints.
  1. The Pie chart off the screen is a problem that could easily be fixed if the box code that displays the chart was actually in the Sprint extension. Instead, it is part of a Phabricator class called PHUIObjectBoxView. Again, it is a question of forking more code and bringing it into the Sprint extension. More upstream code, more maintenance.
  1. The error from Data Tables is a null result set for events in the time frame of the current sprint, which should never happen unless the start date is moved ahead of the task assignments to the project. This can be fixed programmatically, but it is a rather unusual case. The error is not fatal at any rate, and can be rectified simply by doing something to the Sprint, (like moving a task in the workboard).
  1. The inclusion of epoch dates in the CSV file is a side-effect of needing epochs to sort the dates in the data tables. These columns are hidden. There is no way to filter hidden columns from the CSV output currently.
  1. For the "Burn Up" chart, see https://phab08.wmflabs.org/project/sprint/report/burn/?project=PHID-PROJ-ji6y6i7exlyz5xiv2e4c as an example. The Burn Up chart is a different kind of analysis view from the Burn Down. It is a modified version of the Maniphest Report that shows the progression in the ratio between Open and Closed tasks, but does not deal with points. Useful? I am not convinced, but says something, I guess.

I should add that there is one other option for the Sprint creation form and that is to just show the sprint date fields by default. I am not sure that this is a good solution because it may cause more confusion on an already large form. But, it is very simple to implement. If someone sets a date field on a non-sprint project, it has no effect and would not cause any problems for these projects. Please advise if this is feasible.

@Christopher: I'd be ok with having the dates on the default form but that isn't really my call.

@Christopher: regarding ongoing maintenance, I think it would be better to fork (and rename) any css or other resources which are likely to change upstream.

I'm sure you are well aware of all of this but I'll say it anyway:

If you just use existing styles and php classes, upstream changes to those elements will break Sprint. If you fork them, although they might become out of date, at least you won't have a dependency on something that can change at any time. Couple that with phabricator's rapid evolution and it's likely to bite us sooner than later.

So really, in my opinion, forking some code is actually preferable to extending or referencing phabricator core features - as long as the forked code can coexist with updated upstream code, which should not be an issue as long as you are mindful of naming - isolate by prefixing the names of things.

@Christopher: I'd be ok with having the dates on the default form but that isn't really my call.

Same here: Would be fine and less confusing I'd say (though I've now pinged many times folks who created Sprint projects without settings dates which triggered an error on the Sprint list page, but for new sprint project creators showing by default is still better).

Wondering whether the two "Sprint Start Date" and "Sprint End Date" elements could be displayed but disabled (greyed out) by default and get only enabled when the "Is Sprint" checkbox is checked?

chasemp added a comment.EditedApr 22 2015, 2:04 PM

This blocking task has not been resolved to facilitate an upgrade.

Pursuant to:

Assigning to Kristen, per discussions in the TPG weekly meeting where we concluded that she is uniquely qualified to do this specific testing.

any luck here? FYI we will plan on the 22nd but without sign off will delay. :)

With regard for:

  • I would strongly recommend against releasing this as it is currently implemented for the following reasons:

It seems the issue of maintenance and best outcome here doesn't have consensus. I don't feel comfortable upgrading at this time and I believe @Aklapper and I have agreed delay is necessary.

@KLans_WMF and @Awjrichards could you guys weigh in on where you want to go from here? I'm not sure if https://phabricator.wikimedia.org/T95469#1223742 explains things to a point where upgrading and dealing with the limitations makes sense. If possible it would be advantageous to upgrade this week if we are going to do it. AFAIK the majority of the new-ish issues are with new-ish features that we could ignore if we choose?

Anyways, please advise :)

Default show Sprint Start and Sprint End date fields on Project Create form is available for test and review on https://phab08.wmflabs.org/project/create/

@Aklapper the fields are gray until the checkbox is activated. WIthout a js function, I do not think I can make them all activate at once. Also please note that this form is not in the Sprint extension. The method used to render the custom fields on this form is by proxy, not by any changes to the upstream controller.

Also, phabricator version has been updated to Mon, Apr 20, 11:43 d8ab5f594c7f9cd05e333f507f2ae26e55c6b411. No problems have been observed with the Sprint extension and this version. There is a new Batch Edit feature accessed from the workboard column header. Note that batch editing from the workboard will return the viewer back to the default project context workboard, and not the Sprint Board...

@Christopher Default show Sprint Start and Sprint End date fields on Project Create form is a great enhancement. I'm hoping this will save @Aklapper some hours of chasing people down to ask them to set start and end dates :P As Andre described, activating the greyed out "Sprint Start Date" and "Sprint End Date" elements when the "Is Sprint" checkbox is checked would be ideal, but as implemented it is still an improvement. Seems to work well and I'd be happy to see it released to production!

After the creation step, I did notice that clicking on /tag/my-tag/ (e.g. the green thing here: https://phab08.wmflabs.org/tag/magical-sprint-of-dreams/) just appears to reload the page. Maybe related to https://phabricator.wikimedia.org/T95191? Anyway, I would expect the behavior when clicking the green tag box thing from a project page (e.g. https://phab08.wmflabs.org/project/sprint/profile/45/) to be the same as when you click the sprint board icon on the left side of the page, that is you proceed through the workboard setup workflow. Maybe a new feature/out of scope.

Re: go/no-go on upgrading...I'm pretty uncomfortable making that call or even advising one way or another. In the absence of a person who is making decisions on product/user-facing things, I can only give you pros and cons. It seems like @mmodell has a pretty good handle on the code-level and maintenance issues to consider. From the user perspective, the thing that's giving me the most pause is the new Sprint creation form. At best, it will give a tiny number of power users who know about it a fairly inconvenient way to automate a couple steps of sprint creation. At worst, it confuses people and becomes a piece of UI cruft in Phab. If it were to be released, I would recommend that a follow-on task be created to 1.) make it more user friendly and 2.) integrate it in to someplace in the UI that will be more useful to users. Another option would be to hold the sprint creation form feature back from this release and work on the solution to add the functionality that it provides to the default Project creation form (following the path of forking the controller, though as Christopher points out, that has additional maintenance costs).

So I don't think I've directly answered any questions, but some more food for thought at least.

I am remiss in not saying thank you @Chrsitopher for your work here :-)

@KLans_WMF thank you kindly for your help.

I added a few more options to relieve the aggravation of the "mandatory" start and end dates. Rather than force this issue and throw a red exception for the whole world to see, there are now defaults values if the project creator does not want to or forgets to enter them. Start date is set to the creation time and end date is set two weeks from then. This eliminates the problem with the Sprint list and the bother to @Aklapper. Also, I fixed the empty events issue that you noticed, if there are no events, the table will not render.

As far as the issue with clicking on the tag returning to the profile page, this is expected if the board has not been initialized. If there is a board, clicking on the tag takes you there, but if it is not then it takes you to the profile.

If people are OK with these fields on the project form, and the other changes, please advise and I will merge them into the production branch and make a new tag.

KLans_WMF closed this task as Resolved.Apr 29 2015, 7:20 PM
KLans_WMF moved this task from In Progress to Done on the Team-Practices (This-Week) board.

@Christopher: go ahead and merge + tag. I will try to get this published on Wednesday.

@mmodell, OK. Sprint is current with D12603 28.04.2015 upstream. Have you decided what upstream commit will be used?

greg moved this task from INBOX to Done on the Release-Engineering-Team board.May 23 2015, 2:14 PM
Payal.tiwari set the point value for this task to 2.Dec 16 2016, 9:13 AM