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
Introduce uninstalling #67
Conversation
@DeepDiver1975 This requires the API to return status codes outside the 200 range for it to work! |
src/App.vue
Outdated
@@ -12,6 +12,10 @@ | |||
import Navigation from './components/Navigation.vue'; | |||
|
|||
export default { | |||
mounted: function () { | |||
this.$store.dispatch('FETCH_APPLICATIONS') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm... No? It's not used there so it should not be fetched there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "problem" here is, that whenever we change views, the applications will be fetched again and again and again and we don't want that.
App.vue will be mounted only once and whether we start in the list, the detail or the update view, so we can minimize API calls.
Re-fetching will occur after application processes such as installing, uninstalling and updating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Than use created, but this approach is totally wrong. Edit: And additionally to that some timestamp to refetch it if it's too old.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-fetching will occur after application processes such as installing, uninstalling and updating.
And this shows a poor API design :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please merge for now and change Frontend when API is existing.
No reason for a blocker.
fdea359
to
8e404da
Compare
Also includes: