Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new version notification not showing #2716

Closed
rade opened this issue Jul 14, 2017 · 6 comments
Closed

new version notification not showing #2716

rade opened this issue Jul 14, 2017 · 6 comments
Assignees
Labels
bug Broken end user or developer functionality; not working as the developers intended it component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer
Milestone

Comments

@rade
Copy link
Member

rade commented Jul 14, 2017

A long time ago - in #1366 - we introduced an indicator in the app, alerting users to the availability of a new version.

Alas it's not working (anymore).

The api call returns the required info...

$ curl -s http://localhost:4040/api | jq '.'
{
  "capabilities": {
    "report_persistence": false
  },
  "hostname": "xps",
  "id": "4bcc89dcaf3d8115",
  "newVersion": {
    "downloadUrl": "https://github.com/weaveworks/scope/releases/download/v1.5.1/scope",
    "version": "1.5.1"
  },
  "plugins": [],
  "version": "1.5.0"
}

but this is not shown in the UI.

@rade rade added bug Broken end user or developer functionality; not working as the developers intended it component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer labels Jul 14, 2017
@rade rade added this to the Next milestone Jul 14, 2017
@rade
Copy link
Member Author

rade commented Jul 14, 2017

bonus points for figuring out when this got broken.

@rndstr
Copy link
Contributor

rndstr commented Jul 17, 2017

@rade I took a quick look over where this might have stopped working but I cannot see how this ever worked? The received update info newVersion from the API has never been passed to the react/redux store (in

export function receiveApiDetails(apiDetails) {
return {
type: ActionTypes.RECEIVE_API_DETAILS,
hostname: apiDetails.hostname,
version: apiDetails.version,
plugins: apiDetails.plugins
};
}
). Maybe I'm missing something but I also checked out that merge commit of #1366 but it didn't show up as in the posted screenshot.

@rade
Copy link
Member Author

rade commented Jul 17, 2017

Hmm. the screenshot in #1366 immediately follows @davkal's commit of the UI piece, so I would have thought that the former was produced by the latter. Perhaps there's some extra commit missing.

@davkal
Copy link
Contributor

davkal commented Jul 18, 2017

The migration from Flux to Redux (96aae9b) broke this in two ways:

  1. The action on receiving the newVersion item does not set the new version in the store.

  2. The getting of the new version from the redux store cannot work :

96aae9b#diff-63537513399135a955b9e5b988a617b5R44
(newVersion is merged into state and hence becomes an immutable.js object, with .get('prop') accessors instead of .prop.

@rndstr
Copy link
Contributor

rndstr commented Jul 18, 2017

@davkal I thought this as well at first (Flux to Redux seemed to be the culprit) but then I saw that with Flux, receiveApiDetails() was called which did not forward newVersion to the store.

Even though the store dispatcher would've set the newVersion but I don't see how it could be in the payload.

Anyway, I presume I'm missing something somewhere since you (and Tom) both posted screenshots which showed it working.

@davkal
Copy link
Contributor

davkal commented Jul 19, 2017

Just tried the actual PR commit when the feature was introduced. It did not work (and can't, by looking at the action creator). The only explanation for why the feature PR has a working screenshot is that it was mocked up in the footer component.

@rade rade modified the milestones: Next, 1.6 Jul 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken end user or developer functionality; not working as the developers intended it component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer
Projects
None yet
Development

No branches or pull requests

3 participants