Thursday, August 31, 2017

The internship wraps up today, and I decided to wrap things up by livening up the AppDB home page with a screenshot carousel.

Normally I'm not terribly interested in purely cosmetic changes; all the previous changes to the layout that I've made have been to improve usability. But the old home page was afflicted with a sad little jpeg surrounded by too much whitespace at the upper right corner. It just was crying out for something better, and the AppDB has an abundance of screenshots.

Bootstrap made setting up the carousel fairly easy, though there were some minor glitches before I got the surrounding text to behave itself. The main problem then was image sizing.

The sceenshots in the AppDB come in many different sizes and aspect ratios. The carousel would display the full width of the screenshots, but size itself up or down as needed to keep the proportions. The result was a very unpleasant jumping up and down of the text below the carousel as it resized itself.

Limiting the height of the carousel and setting overflow to hidden stopped the jumping, but it meant that taller screenshots were all arbitrarily cropped at the bottom, and the results weren't usually pleasing.

Ultimately I decided to select an assortment of screenshots from the AppDB and manually edit them to uniform proportions for the carousel. The results went live a few minutes ago, as I write this.

This will be my last blog post for awhile; there will be one more to report on WineConf2017, which I'll be attending at the end of October. This isn't, however, the end of my work on the AppDB. I have a list of things I still want to do.

Saturday, August 19, 2017

This past week I finally got around to fixing the very first bug I ever filed against the AppDB.

It wasn't really a bug per se, but an enhancement request: add a field to the test report form for changes to default settings in winecfg (Wine's configuration tool). The bug has been gathering  dust in Wine's bugzilla since 2008, but the reasons for wanting the enhancement are still valid.

The AppDB ratings systems differentiates between applications that work as well as on Windows  "out-of-the-box" (platinum rating) and ones that can be made to work as well as on Windows with some effort (gold rating). False platinums submitted by users who did in fact use various tweaks to get the application working has been a chronic problem, as has the issue of users who give gold ratings but fail to specify the workarounds needed to achieve that.

I was already familiar with the test report form code from having grappled with the distributions issue, so adding another field to the form and database would not be a difficult task. The challenges here were in the design and wording of the interface: my goal was to reduce user errors, not create more.

The first issue was what to name the field. My bug report just mentioned changes to winecfg, but in reality the kinds of things that might need to be done to make an application work  encompassed more than that, such as changes to registry settings or installation of runtimes or codecs. In addition, many users make such changes through winetricks, a third party script, and don't think of them as changes to winecfg.

I needed a broader word that would encompass all possible fiddling, and in the end it came down to a choice between "tweaks" and "workarounds." I chose the latter because that's the word most commonly used in bugzilla.

The next issue was deciding what kind of field to add. I knew we needed a wysiwyg textarea that would allow users to say whatever they wanted/needed to say, but I also knew from similar fields already in the form that such a field is not useful for restricting ratings precisely because users can say whatever they want. If users could simply type in "None" in a Workarounds field, then the presence of content in that field couldn't reliably be used to restrict platinum ratings.

So I decided to add two fields to the Workarounds section. One is a simple yes/no question with radio buttons: "Were any workarounds used for problems that do not exist in Windows?"  The other is a wysiwyg textarea for users who answer "yes" to describe the workarounds they used.

With that in place, I added checks to the form processing to make sure the submitted rating is consistent with the answer to the Workarounds question. Users are now prevented from submitting platinum ratings if they report having used workarounds, and users who submit gold ratings without having described the workarounds used are now required to supply that information.

Could users willfully bypass those checks by, for example, putting workarounds information in another field and giving a platinum rating? Sure. However, I don't believe many (if any) of the incorrect ratings we've seen have been willful attempts to mislead, and if there are any, they will still be caught by the maintainer or admin processing the submitted form.

Sunday, August 6, 2017

For the past couple of weeks I've been focusing on changes that mainly benefit AppDB admins, specifically the management of maintainers.

One of the first problems I fixed when I began this internship was with the interface for managing maintainers. In addition to fixing the sort order to something that made sense to humans (the original order was by userId, and appeared random), I also added a column to display the last time the maintainer had logged into the AppDB. I suspected that we had many maintainers who hadn't logged in in a long time, and the actual number proved even worse than I had feared: 57% of the maintainerships belonged to a user who hadn't logged in in over two years. (Note the number is for maintainerships, not users, as one user may have multiple maintainerships.)

How did this happen? The function that notifies maintainers of new test reports in their queue also sends two  reminders, and removes the maintainership if they fail to process the queued reports after 7 days. That worked well for widely-used programs, but the AppDB has many entries for less-popular programs. Those were the maintainers falling through the cracks: since they weren't getting submitted test reports, and nothing was checking to see if they were submitting their own (which they are supposed to do), they were never removed.

About a month ago I removed the inactive user check, and in the process of studying that code, discovered that it was supposed to also remove maintainerships from users who had not logged in in six months. It never worked, because the check was only run against users who did not have data associated with their accounts, and being a maintainer was defined as having data. So the check was looking for maintainers in an array that had explicitly excluded them.

With all that in mind, I added a control to identify and delete maintainers who had not logged in in over 24 months to the admin control center. I eventually plan to add it to the daily cleanup cron script. The time period, IMO, is still overly generous; I selected it in part because of the large backlog of maintainerships that were going to be removed the first time it was run (3439 out of 6242), and in part because no time limit had ever been enforced before and I wanted to make it exceedingly difficult for anyone to argue that the definition of "inactive" was unreasonable. I may eventually shorten the time period to 18 or even 12 months.

Sunday, July 23, 2017

After fixing some smaller bugs, I returned to the problem of distributions still being added to the database even when the admin or maintainer rejects them in the queue.

I decided to address the problem in smaller steps this time rather than trying to fix it all at once. The first step was to submit the patch that did at least separate the test report from the invalid distribution when it was changed in the queue. Even though it didn't fully fix the problem, it  made things much easier for admins, in that all we had to do now was delete the distribution that shouldn't have been added, whereas before we had to first track down the incorrect test report and change the distribution in it again before being able to delete the invalid one.

I then shifted my focus to why users were entering invalid distributions in the first place. I found a number of problems with the UI that made user errors more likely.

The first was the word "distribution" itself. That's a Linuxism that makes no sense for users of other operating systems, (Wine has a lot of macOS users), and many new Linux users don't understand the term either.  A lot of the invalid "distributions" I saw submitted in test reports were actually names of distributors. So I changed "distribution" to "operating system" in areas visible to users, and rephrased the help text to make it clearer that users should look for theirs in the dropdown list first and only add a new one if it is not found.

The next thing I looked at was the notice presented in the queue to admins/maintainers when a user had added a new operating system: it simply wasn't noticeable enough. I added a warning icon and background color to make it stand out more against the rest of the form.

After that I took another look at stopping rejected items from being added to the database. The real problem is that there is one submit button at the bottom of the queued form, and pressing that leads to everything in the form being accepted. There was nothing in the code that explicitly rejected a submitted operating system when it was replaced with one in the dropdown list, and that's what needed to be added.

The code already had a check for whether the operating system had been changed from the original submission, though all it did was add a message that x had been changed to y. That seemed the logical place to force the system to delete the submitted operating system if it had been changed and the original one wasn't accepted, but finding the right way to do it proved a bit tricky. Simply checking the state of the submitted operating system turned out to be a bad idea because the state was changed to accepted as soon as that submit button was clicked. I eventually got it to work as it should by checking the count of test reports attached to the operating system, and forcing a delete it if there was no more than one.

It's now working as it should, but I'm not entirely happy with it. I've patched up the visible problems, but IMO the underlying design is still flawed.

Sunday, July 9, 2017

My work in the past couple of weeks included what is probably the most visible change I have made to the AppDB UI: using collapsible panels on the versions pages.

The versions pages are probably the most-used part of the AppDB. It's where users go to find out how well a particular application or game runs, whether there are any known bugs, and what, if anything, they need to do to get it running in Wine. There are four main sections: test results, known bugs, howtos/note, and comments.

Heretofore, the contents of all four sections were fully displayed on entering the page. It looked  messy and was difficult to navigate. New users often didn't realize there were howtos on the page because they didn't bother to scroll down, and even users who knew what they were looking for had to hunt for it in the clutter.

A forum user had suggested using collapsible panels on that page several months ago. I thought it was a good idea, so I decided to take a stab at implementing it.

Adding the panels was easy using Bootstrap; making them behave exactly as I wanted was a bit trickier. The panel headings were now links, and showed up as underlined, which didn't look very nice. I also wanted to add some sort of icon that would toggle with the open/closed state, but my initial attempts made a mess of the heading layout. My mentor gave me a couple of great suggestions for solving those two problems.

That left me with one remaining problem: both the test results and bug links tables had buttons that toggled the display between showing some (the default) or all of the tests/bugs. Clicking on those buttons would refresh the page, causing the panels to collapse. As a user, I'd consider that a major annoyance.

Googling the problem suggested using either cookies or localStorage to retain the state of the panel when the page was refreshed. I opted for the latter, and after some experimenting with  sample scripts, was able to adapt one to the specific needs of the AppDB.

The change was committed, and so far is working well. I'm pleased with the result, and the user who suggested it thanked me.

Sunday, June 25, 2017

After adding a checkbox to the test report form to differentiate staging releases from the development branch went fairly smoothly, I decided to take a stab at a longstanding and (for AppDB admins) very annoying bug: new distributions are added to the database even if the admin rejects them.

The test report form users fill out has a dropdown list from which they can select their operating system (though it's labeled "Distribution," which is in itself a problem since we also have MacOS and *BSD users). There is also an input box for users to add their operating system/distribution if they don't find it on the list, and that's where the problem starts. Some users don't search very carefully and enter distros that are already on the list, while others misunderstand the Distribution label completely and try to enter the name of the developer, distributor, or the application version.

Whatever the mistake, maintainers and admins are supposed to be able to correct it when processing test reports, and that's what's not working. The person processing the queued form can delete the added distribution from the input box and select one from the dropdown list, but that change is ignored when the test report is approved and the invalid distro is added to the database, with the test report still attached to it.

So I spent the past week and a half beating my head against the wall trying to figure out why this is happening. Much of that was focused on comparing the code for submitting new distributions to the code for submitting new vendors, which has the same UI but is working properly. I  tried changing parts of the distribution code to mimic the way it was done in t he vendor code. Changing the way the dropdown list is generated made a partial difference, and I do have a patch that enables the test report to be submitted with the changed information. Unfortunately, the incorrect distro is still added to the database (albeit with zero tests attached, so it's easier to clean up) so I know it's not completely correct.

At that point I decided to take a step back and work on something else for awhile. Not giving up; I'll come back to it with fresh eyes and a bit more experience.

On the plus side, the first thing I turned my attention to was another old bug turned out to be simply a typo in a URL.

Saturday, June 10, 2017

I come into this internship with an extensive knowledge of Wine and the AppDB as a user (9 years) and little in the way of technical skills. What I know is what I've picked up over the years as a Linux user, which, granted, is much more than what I would have learned had I stayed with Windows.

But in terms of what's needed to wrestle with the AppDB code, it's not much: basic html and css, an understanding of how relational databases work from a job years ago where I had to run queries using a now-defunct language on an IBM mainframe, and (thanks to having run regression tests on Wine) how to use git. I had no training or experience in Apache, MySQL, PHP, Javascript, JQuery, or Bootstrap prior to being accepted as an intern.

I spent the month leading up to the official start of the internship setting up a local test environment, and with the help of a mentor who's been very patient with my newbie questions, getting better at troubleshooting Apache and PHP errors.

The learning curve so far has been about what I expected. MySQL has been the easiest because I do understand how database queries work and just have to look up the specific MySQL syntax for what I want to do. PHP has been the hardest, in part because the AppDB code is poorly organized and difficult to follow even for someone who knows PHP. 

I have nonetheless managed to fix some small bugs, and learned a few things. I expect the learning curve to get steeper before it gets easier.