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

DB: support "best price search" #314

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

derhuerst
Copy link
Member

@derhuerst derhuerst commented Mar 20, 2024

@bergmannjg I went ahead and created a WIP PR.

Fixes #291.
Fixes juliuste/db-prices#33.
Fixes derhuerst/db-prices-cli#4.

index.js Show resolved Hide resolved
@bergmannjg bergmannjg force-pushed the add-BestPriceSearch branch from ce4b5b5 to 92712c7 Compare March 23, 2024 11:14
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
const {res, common} = await profile.request({profile, opt}, userAgent, {
cfg: {polyEnc: 'GPA'},
meth: 'BestPriceSearch',
req: profile.transformJourneysQuery({profile, opt}, query),
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a heads-up: There might be a case – that I can't think of right now though – that fails because transformJourneysQuery() assumes a TripSearch call.

index.js Outdated Show resolved Hide resolved
parse/bestprice.js Outdated Show resolved Hide resolved
parse/bestprice.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
parse/bestprice.js Outdated Show resolved Hide resolved
@derhuerst derhuerst requested a review from juliuste May 5, 2024 19:03
throw new TypeError('opt.departure is invalid');
}
const now = new Date();
let today = DateTime.fromObject({
Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, even though I told you to do it this way, I think this isn't correct either: For example, if profile.timezone is Europe/Berlin, the timezone-naive now is 2024-05-06T00:30+01:00 because it runs on a machine in Europe/London, then this will create a Berlin-timezone timestamp for the next day.

I can think of a least two options, the latter being the more straightforward one IMO:

  1. Change now to be timezone-aware, so that now.{year, month,day} are correct.
  2. Just use one chain of DateTime calls, as illustrated below.

Also, while implicitly casting today to a number below works, it would be cleaner to add .toMillis().

const today = DateTime
	.fromMillis(Date.now(), {
		zone: profile.timezone,
		locale: profile.locale,
	})
	.startOf('day')
	.toMillis()

@derhuerst
Copy link
Member Author

remotely related: #331

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

Successfully merging this pull request may close these issues.

DB: support "best price search" Invalid JSON response. Bahn API change? doesn't work
2 participants