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

[O2B-532] Use time range filter for run start stop #1482

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

martinboulais
Copy link
Collaborator

I have a JIRA ticket

  • branch and/or PR name(s) include(s) JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected

Notable changes for users:

  • Use time range picker for runs start & stop filtering

@martinboulais martinboulais self-assigned this Mar 28, 2024
@martinboulais martinboulais force-pushed the feature/O2B-532/use-time-range-filter-for-run-start-stop branch from 7d740bf to 30e512c Compare April 10, 2024 06:26
@martinboulais martinboulais force-pushed the feature/O2B-532/use-time-range-filter-for-run-start-stop branch 3 times, most recently from 959ebe0 to 0b8afc8 Compare April 22, 2024 06:31
@martinboulais martinboulais force-pushed the feature/O2B-532/use-time-range-filter-for-run-start-stop branch from 0b8afc8 to c18389f Compare May 21, 2024 14:10
@martinboulais martinboulais force-pushed the feature/O2B-532/use-time-range-filter-for-run-start-stop branch from 7ce8761 to 350e886 Compare May 22, 2024 09:28
Comment on lines 14 to 18
import { timeRangeFilter } from '../common/filters/timeRangeFilter.js';

let today = new Date();
today.setMinutes(today.getMinutes() - today.getTimezoneOffset());
[today] = today.toISOString().split('T');
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably debug garbage, removed

Comment on lines 188 to 189
ret.date = formatTimestampForDateTimeInput(min + MILLISECONDS_IN_ONE_DAY, this._seconds).date;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe minTimestamps or something instead of min, the same with max

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, done

Comment on lines 212 to 213
const ret = {};

Copy link
Collaborator

Choose a reason for hiding this comment

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

abbr.

}
if (this.o2endFilterTo !== '') {
this.activeFilters.push('O2 End to');
}
if (this._runDurationFilter !== null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method is dead code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, deleted

Comment on lines +735 to 738
* @return {EorReasonFilterModel} the run type filtering model
*/
get eorReasonsFilterModel() {
return this._eorReasonsFilterModel;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that you can also rename the field to singular

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just fixed the docblock, this would be a bit too much refactoring

Comment on lines 715 to 717
/**
* Return the selector for all the inputs composing a period inputs selectors
*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Return selectors for all inputs composing a period input ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants