Skip to content

Add date range, declared type, description for news#49

Merged
GormFrank merged 1 commit intoServiceCanada:mainfrom
ipaksc:Daterange
Oct 10, 2025
Merged

Add date range, declared type, description for news#49
GormFrank merged 1 commit intoServiceCanada:mainfrom
ipaksc:Daterange

Conversation

@ipaksc
Copy link
Contributor

@ipaksc ipaksc commented Jul 23, 2025

No description provided.

Copy link
Contributor

@GormFrank GormFrank left a comment

Choose a reason for hiding this comment

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

@ipaksc Your PR causes bizarre conflicts with the upstream, it would be best to either create a new PR with just the changes from your last commit (to src/connector.js, test/newsadv-en.html and test/newsadv-fr.html) or to copy the files from your last commit then reset hard to the upstream/main and then do a new single commit with your files and push to your Daterange branch which will erase and override your 4 other commits included in this PR that are unnecessary.

@GormFrank GormFrank changed the title Daterange Add date range, declared type, description for news Sep 26, 2025
Copy link
Contributor

@GormFrank GormFrank left a comment

Choose a reason for hiding this comment

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

@ipaksc Just a small change with the mapping of values. Also, please review your form's HTML in accrodance to Bootstrap 3, since the fields look all stuck together (there is a lack of breathing space).

@GormFrank
Copy link
Contributor

@ipaksc Maybe you could look at for the form alignment: https://getbootstrap.com/docs/3.4/css/#forms-horizontal

Copy link
Contributor

@GormFrank GormFrank left a comment

Choose a reason for hiding this comment

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

@ipaksc Small changes to bring back things I asked you to remove. But I tested the logic in various Canadian time zones (for dates) and everything is good. So once the little changes are made, I will merge.

@GormFrank
Copy link
Contributor

GormFrank commented Oct 9, 2025

Tests conducted

On the News test page provided:

  • 1. Do a search with a keyword - Expected: Search resuts returned in relations to the keyword, and the description of the news item is shown instead of the usual generated excerpt that showcases the matching keywords on the page.
  • 3. Select an institution and search - Expected: Search results are strictly limited to that institution.
  • 4. Select a News type and search (in combination with previous) - Expected: Search results are strictly limited ti that type.
  • 5. Select start date and search - Expected: Search results returned have a date of at least (>=) the start date entered in the input.
  • 6. Select end date and search (in combination with start date) - Expected: Search results returned have a date of at max (<=) the end date entered in the input.
  • 7. Change timezone and test dates again - Expected: Returned results should still be within the boundaries entered in the input type dates.

@GormFrank
Copy link
Contributor

GormFrank commented Oct 9, 2025

@ipaksc Discussion needed!

We need to determine if we are taking the resultDate or a new field for date issued to limit the date with the inputs.

UPDATE: This will be solved through the Coveo platform directly.

@GormFrank GormFrank dismissed igkislev’s stale review October 10, 2025 20:01

Changes sorted out.

@GormFrank GormFrank merged commit 7acd437 into ServiceCanada:main Oct 10, 2025
1 check passed
@GormFrank GormFrank added this to the v1.7.0 milestone Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants