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

✨ OSIDB-3562: Flaw history filtering #477

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

Conversation

C-Valen
Copy link
Member

@C-Valen C-Valen commented Nov 11, 2024

OSIDB-3562: Flaw history filtering

Checklist:

  • Commits consolidated
  • Changelog updated
  • Test cases added/updated
  • Jira ticket updated

Summary:

Implementation of date range filter on the flaw history.

Changes:

  • Add support for history item filtering
  • Allow custom placeholders on EditableDate
  • Add date inputs for date range filtering
  • Add button to clear filters
  • Update FlawHistory test cases and snapshots

Considerations:

Closes OSIDB-3562

@C-Valen C-Valen added the enhancement New feature or request label Nov 11, 2024
@C-Valen C-Valen self-assigned this Nov 11, 2024
@C-Valen C-Valen requested a review from a team November 11, 2024 12:20
@C-Valen C-Valen marked this pull request as ready for review November 11, 2024 12:24
Comment on lines 20 to 23
let subject: VueWrapper<InstanceType<typeof FlawHistory>>;

osimEmptyFlawTest('is not shown if no history present on flaw', async ({ flaw }) => {
const subject = mount(FlawHistory, {
subject = mount(FlawHistory, {
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?
This is the only test that uses the top level subject

Copy link
Member Author

Choose a reason for hiding this comment

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

That was probably missed from a reverted change I tried. I'll correct it.

@C-Valen C-Valen force-pushed the feature/OSIDB-3562-flaw-history-filtering branch 2 times, most recently from 461164b to 7223c06 Compare November 14, 2024 14:57
return !startDate.value && !endDate.value;
});

const validDateRange = computed(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const validDateRange = computed(() => {
const isDateRangeValid = computed(() => {

Comment on lines 41 to 42
const start = new Date(startDate.value!);
const end = new Date(endDate.value!);
Copy link
Collaborator

@superbuggy superbuggy Nov 14, 2024

Choose a reason for hiding this comment

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

specify time boundaries here: for start use 00:00 UTC and for end 23:59 UTC

return itemDate.toDateString() === start.toDateString();
} else {
// Otherwise, return items within the range
return itemDate >= start && itemDate <= end;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return itemDate >= start && itemDate <= end;
// if you have end date be one day ahead of the user selected end date
return itemDate >= start && itemDate < end;

MrMarble
MrMarble previously approved these changes Nov 14, 2024
src/components/FlawHistory.vue Outdated Show resolved Hide resolved
src/components/FlawHistory.vue Outdated Show resolved Hide resolved
Comment on lines 49 to 51
if (start.toDateString() === end.toDateString()) {
// Check if the item's date is on the exact day of start and end
return itemDate.toDateString() === start.toDateString();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (start.toDateString() === end.toDateString()) {
// Check if the item's date is on the exact day of start and end
return itemDate.toDateString() === start.toDateString();
if (start === end) {
// Check if the item's date is on the exact day of start and end
return itemDate === start;

Copy link
Member

Choose a reason for hiding this comment

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

This widget is lacking tests, maybe is a good opportunity to add them, at least to test the new functionality

MrMarble
MrMarble previously approved these changes Nov 19, 2024
CHANGELOG.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Fix the changelog after release merge

♻️ Use history as component property instead whole flaw

✨ Allow custom placeholder
stubs: {
EditableDate: true,
},
},
});
const historyListItem = subject.find('li div');
expect(historyListItem?.text()).includes('Update Owner:');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding tests for the date range filtering would be awesome

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do that, and also as discussed adding a test for EditableDate making sure that it renders and the property placeholder works as expected.

Copy link
Collaborator

@superbuggy superbuggy left a comment

Choose a reason for hiding this comment

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

We could add tests for date range filtering which would allow us to be confident the filtering is working as intended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants