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.

Sign up for GitHub

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

Jump to bottom

Check repo state instead of props when upgrading #1469

Merged
merged 2 commits into from Jan 20, 2020
Merged

Check repo state instead of props when upgrading #1469

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23d18f7
Check repo state instead of props when upgrading
Jan 17, 2020
14d2ee6
Fix test
Jan 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion dashboard/src/components/AppUpgrade/AppUpgrade.tsx
View file
Open in desktop
Expand Up @@ -61,6 +61,7 @@ class AppUpgrade extends React.Component<IAppUpgradeProps, IAppUpgradeState> {

public componentDidUpdate(prevProps: IAppUpgradeProps) {
const { repos, app } = this.props;
const prevRepo = this.state.repo;
let repo = this.state.repo;
// Retrieve the current repo
if (!repo) {
Expand Down Expand Up @@ -93,7 +94,7 @@ class AppUpgrade extends React.Component<IAppUpgradeProps, IAppUpgradeState> {
repo &&
repo.metadata &&
repo.metadata.name &&
(prevProps.app !== app || prevProps.repo !== repo)
(prevProps.app !== app || prevRepo !== repo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had expected the test you added with 7b96684 to fail with this change, as it was added to verify that if the props.repo changes it fetches the data? Why is it not failing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that test, state.repo is empty at the beginning. Then component gets updated with props.repo. componentDidUpdate is executed and the state is updated. Since prevRepo was empty and repo now contains the repo info, getDeployedChartVersion is executed.

) {
const chartID = `${repo.metadata.name}/${chart.metadata.name}`;
this.props.getDeployedChartVersion(chartID, chart.metadata.version);
Expand Down