Empty Label response can possibly be cached from GitHub?

[miketaylr]

Sadly our existing labels tests aren't good enough. This came up trying to get #712 working.

Right now on production if you open the label gear you get an empty label w/ the following error in the console: no element found

Looks like we're sending an empty response?

This doesn't happen in dev, but the other bug is that if you open the label editor on an issue with labels, the checkboxes aren't selected. That's a regression, likely from all the namespace mangling.

I'm beginning to want to rip out all the special namespace handling code and just show them everywhere.

@karlcow any good reasons to not show browser-foo and status-foo in the webcompat UI? Or were we just trying to have a seamless transition?

[miketaylr]

I'm going to remove the tests that aren't great, IMO. We should add back the following once we solve the bug:

      'Add and remove a label': function () {
        return this.remote
          .setFindTimeout(intern.config.wc.pageLoadTimeout)
          .get(require.toUrl(url('/issues/2')))
          .findByCssSelector('.LabelEditor-launcher').click()
          .end()
          .findByCssSelector('label.LabelEditor-item input[name="contactready"]').click()
          .end()
          .sleep(200)
          .findByCssSelector('.Label--badge').getVisibleText()
          .then(function (labelText) {
            assert.include(labelText, 'contactready', 'Label appears once it has been selected');
          })
          .end()
          .findByCssSelector('.LabelEditor-launcher').click()
          .end()
          .findByCssSelector('label.LabelEditor-item input[name="contactready"]').click()
          .end()
          .findByCssSelector('.LabelEditor-btn').click()
          .end()
          .sleep(200)
          .findByCssSelector('.Label--badge')
          .getVisibleText()
          .then(function (labelText) {
            assert.include(labelText, 'contactready', 'Label has been removed');
          })
          .end();
      }

We can also add python unit tests at the API level around adding and removing labels. That can be handled in #396.

[miketaylr]

It seems like if we move to a vertical label editor (cf. #545), having the extra status- or browser- text won't be a big deal.

[miketaylr]

Hmm, the "no element found" bug doesn't reproduce for me all of the sudden. But the un-checked label checkbox reproduces 100%.

Screenshot to prove I'm not crazy:

[karlcow]

@miketaylr

any good reasons to not show browser-foo and status-foo in the webcompat UI? Or were we just trying to have a seamless transition?

The namespace is to allow us to separate them in the UI. Basically at the end I don't think the label gear (in the future) would have any kind of status- (just browser- etc). Note also that if you rip the namespace, you will get a mix-order of things. It's already messy currently. Without the namespace it will be a big bag of keywords. Not fun.

[karlcow]

Right now on production if you open the label gear you get an empty label w/ the following error in the console: no element found

This was happening to me before the renaming, specifically in Japan. When you open the gear, and github doesn't have time to send the response, and you start playing with it and it erases all labels.

[miketaylr]

Note also that if you rip the namespace, you will get a mix-order of things. It's already messy currently. Without the namespace it will be a big bag of keywords. Not fun.

Ah, OK. So we're relying on the namespace to alphabetize/sort the labels for us?

[miketaylr]

This was happening to me before the renaming, specifically in Japan.

OK, let's ignore that problem for now and focus on:

the other bug is that if you open the label editor on an issue with labels, the checkboxes aren't selected.

[miketaylr]

And now it doesn't reproduce in production or development.

So apologies @karlcow for insinuating you/we broke something here. 🙇

My best guess is something bad happening between GitHub and us. We do cache the labels endpoint, so one theory is somehow we cached an empty response:

https://github.com/webcompat/webcompat.com/blob/master/webcompat/api/endpoints.py#L260-L274

@karlcow should we rip out the caching? I think it's the only place we use that Flask-Cache extension, and it's caused us some problems in the past as well.

[miketaylr]

☑️ My vote is to remove the caching of labels and let the browser use its cache.