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

Fix refresh frequency #7248

Merged
merged 2 commits into from
Apr 9, 2019
Merged

Fix refresh frequency #7248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d0477a
Fix refresh frequency
betodealmeida Apr 9, 2019
f53b24c
Fix unit tests
betodealmeida Apr 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions superset/assets/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ describe('HeaderActionsDropdown', () => {
expect(wrapper.find(MenuItem)).toHaveLength(1);
});

it('should render the RefreshIntervalModal', () => {
it('should not render the RefreshIntervalModal', () => {
const wrapper = setup(overrideProps);
expect(wrapper.find(RefreshIntervalModal)).toHaveLength(1);
expect(wrapper.find(RefreshIntervalModal)).toHaveLength(0);
});

it('should render the URLShortLinkModal', () => {
Expand Down Expand Up @@ -105,9 +105,9 @@ describe('HeaderActionsDropdown', () => {
expect(wrapper.find(MenuItem)).toHaveLength(2);
});

it('should render the RefreshIntervalModal', () => {
it('should not render the RefreshIntervalModal', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add tests that assert it renders this in editMode?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have them already, actually. I didn't have to change them. See https://github.com/apache/incubator-superset/pull/7248/files#diff-c130332fd863f354e0dd6e8e6097f72fR142

const wrapper = setup(overrideProps);
expect(wrapper.find(RefreshIntervalModal)).toHaveLength(1);
expect(wrapper.find(RefreshIntervalModal)).toHaveLength(0);
});

it('should render the URLShortLinkModal', () => {
Expand Down
2 changes: 2 additions & 0 deletions superset/assets/src/dashboard/components/Header.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ class Header extends React.PureComponent {
colorScheme,
filters,
dashboardInfo,
refreshFrequency,
} = this.props;

const scale = CategoricalColorNamespace.getScale(
Expand All @@ -231,6 +232,7 @@ class Header extends React.PureComponent {
label_colors: labelColors,
dashboard_title: dashboardTitle,
default_filters: safeStringify(filters),
refresh_frequency: refreshFrequency,
};

// make sure positions data less than DB storage limitation:
Expand Down
12 changes: 7 additions & 5 deletions superset/assets/src/dashboard/components/HeaderActionsDropdown.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,13 @@ class HeaderActionsDropdown extends React.PureComponent {
<MenuItem onClick={forceRefreshAllCharts} disabled={isLoading}>
{t('Force refresh dashboard')}
</MenuItem>
<RefreshIntervalModal
refreshFrequency={refreshFrequency}
onChange={this.changeRefreshInterval}
triggerNode={<span>{t('Set auto-refresh interval')}</span>}
/>
{editMode && (
<RefreshIntervalModal
refreshFrequency={refreshFrequency}
onChange={this.changeRefreshInterval}
triggerNode={<span>{t('Set auto-refresh interval')}</span>}
/>
)}
{editMode && (
<MenuItem target="_blank" href={`/dashboard/edit/${dashboardId}`}>
{t('Edit dashboard metadata')}
Expand Down
6 changes: 5 additions & 1 deletion superset/assets/src/dashboard/reducers/dashboardState.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,11 @@ export default function dashboardStateReducer(state = {}, action) {
return { ...state, hasUnsavedChanges };
},
[SET_REFRESH_FREQUENCY]() {
return { ...state, refreshFrequency: action.refreshFrequency };
return {
...state,
refreshFrequency: action.refreshFrequency,
hasUnsavedChanges: true,
};
},
};

Expand Down