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

[ACS-4985] Added configuration to merge created and modified date filters into a single date filter #3324

Merged
merged 12 commits into from
Jul 28, 2023
Merged

[ACS-4985] Added configuration to merge created and modified date filters into a single date filter #3324

merged 12 commits into from
Jul 28, 2023

Conversation

swapnil-verma-gl
Copy link
Contributor

@swapnil-verma-gl swapnil-verma-gl commented Jul 10, 2023 •
edited by jira bot
Loading

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)
Created and Modified date filters are separate filters in the ACA UI

What is the new behaviour?
Added configuration to use the new SearchDateRangeAdvancedTabbed component, which allows merging the 2 date filters into a single UI Chip

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:
For more info, visit ACS-4985

@swapnil-verma-gl swapnil-verma-gl marked this pull request as ready for review July 18, 2023 12:03
MichalKinas
MichalKinas reviewed Jul 18, 2023
View reviewed changes
Copy link
Contributor

@MichalKinas MichalKinas left a comment

Choose a reason for hiding this comment

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

Please remember to review and fix e2es related to this ticket excluded in e2e/protractor/protractor.excludes.json. It can be done as part of another PR if you like but in this case please create a Jira for it

DenysVuika
DenysVuika reviewed Jul 18, 2023
View reviewed changes
package.json
@@ -50,6 +50,7 @@
"@angular/core": "14.1.3",
"@angular/forms": "14.1.3",
"@angular/material": "14.1.3",
"@angular/material-date-fns-adapter": "14.1.3",
"@angular/material-moment-adapter": "14.1.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need removing the moment one? or it's still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. I don't see usages of the moment adapter in ACA anymore, but there would still be its usages in ADF. There are usages of the base moment library in some ACA E2E test cases though

@swapnil-verma-gl swapnil-verma-gl force-pushed the ACS-4985-Advanced-Search-Date-picker-facets-Created-and-Modified branch from 03d07af to 1fb49bb Compare July 19, 2023 07:37
@swapnil-verma-gl
Copy link
Contributor Author

swapnil-verma-gl commented Jul 20, 2023 •
edited
Loading

Please remember to review and fix e2es related to this ticket excluded in e2e/protractor/protractor.excludes.json. It can be done as part of another PR if you like but in this case please create a Jira for it

All the impacted e2e test cases from this change are in protractor and require major rewriting to accomodate for the new UI. Since we are migrating over from protractor to playwright, I don't think we need to make any changes to them for now. I have added them to the protractor.excludes.json file for now.

The migration of these test cases can be tracked under ACS-5007

@swapnil-verma-gl swapnil-verma-gl force-pushed the ACS-4985-Advanced-Search-Date-picker-facets-Created-and-Modified branch from 251dd00 to 5468c80 Compare July 26, 2023 06:18
@nikita-web-ua
Copy link
Contributor

I don't get why do we need .env-original ?

@swapnil-verma-gl
Copy link
Contributor Author

I don't get why do we need .env-original ?

We don't need that file. It probably got added by mistake when I was rebasing my branch to be up to date with develop. I've removed it now.

@swapnil-verma-gl swapnil-verma-gl merged commit 174d7b6 into develop Jul 28, 2023
@swapnil-verma-gl swapnil-verma-gl deleted the ACS-4985-Advanced-Search-Date-picker-facets-Created-and-Modified branch July 28, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichalKinas MichalKinas MichalKinas left review comments

@DenysVuika DenysVuika DenysVuika left review comments

@AleksanderSklorz AleksanderSklorz AleksanderSklorz approved these changes

@nikita-web-ua nikita-web-ua nikita-web-ua approved these changes

@eromano eromano Awaiting requested review from eromano

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@swapnil-verma-gl @nikita-web-ua @DenysVuika @MichalKinas @AleksanderSklorz