Skip to content
This repository has been archived by the owner on Sep 11, 2023. It is now read-only.

T1-2846 #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions basket/Basket.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,16 @@ export default class Basket {
// load default start date instance
if (!startDateInstance) startDateInstance = this.getCurrentDateInstance()

// check that selected date and timespan is within available dates
const tmpBasketEntry = new BasketEntry({})
Copy link
Contributor

Choose a reason for hiding this comment

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

why empty object?

tmpBasketEntry.setValidFrom(startDateInstance)
tmpBasketEntry.setProductDefinitionInstance(definition)
const checkResult = this.checkAvailableDates([tmpBasketEntry])
michael-scheurer marked this conversation as resolved.
Show resolved Hide resolved
if (!checkResult) {
EventBus.$emit('spinnerHide')
return false
}

// set standard user data
if (!userData) {
userData = new UserData({
Expand Down Expand Up @@ -215,6 +225,13 @@ export default class Basket {
* @returns {Promise<boolean>}
*/
async updateBasketEntries(basketEntriesArray) {
// check validity dates
const checkResult = this.checkAvailableDates(basketEntriesArray)
Copy link
Contributor

Choose a reason for hiding this comment

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

again, this seems very weird

if (!checkResult) {
EventBus.$emit('spinnerHide')
return false
}

// prepare basket entries for the api
const preparedBasketEntries = basketEntriesArray.map((basketEntry) => {
basketEntry.getUserData().setCompleteForCheckout(basketEntry)
Expand Down Expand Up @@ -285,6 +302,13 @@ export default class Basket {
) {
if (showSpinner) EventBus.$emit('spinnerShow')

// check that selected date and timespan is within available dates
const checkResult = this.checkAvailableDates([basketEntryInstance])
if (!checkResult) {
EventBus.$emit('spinnerHide')
return false
}
Comment on lines +306 to +310
Copy link
Contributor

Choose a reason for hiding this comment

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

lots of duplicate code aswell, you have the same logic 3 times here now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to improve? If the check fails, the function has to exit and return a boolean..

Copy link
Contributor

@fewhnhouse fewhnhouse Nov 13, 2021

Choose a reason for hiding this comment

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

@michael-scheurer it cant be that the logic has to check for this behaviour 3 times in a row. If it indeed does, you should split up the function it is in into 3 separate functions, as it makes it obvious that the current function is handling 3 different usecases at the same time.


// is the basket entry ready to be complete for checkout ?
basketEntryInstance
.getUserData()
Expand Down Expand Up @@ -813,6 +837,40 @@ export default class Basket {
return true
}

/**
* Is the booked time span within the available dates of a product (considering availability range and validity dates)
* @param basketEntries: BasketEntry[]
* @return {boolean}
*/
checkAvailableDates(basketEntries) {
if (!basketEntries || !basketEntries.length) {
throw new Error('No product definitions provided in duration check')
}
const shopModulesInstance = store.getters.getShopModulesInstance()
// iterate product definitions
for (let i = 0; i < basketEntries.length; i++) {
const basketEntry = basketEntries[i]
if (basketEntry.isRequiredEntry() || basketEntry.isEventEntry()) {
// no product would be found
continue
}
const bookingStart = basketEntry.getValidFrom()
const productDefinition = basketEntry.getProductDefinition()
const productId = productDefinition.getProductId()
const productInstance = shopModulesInstance.getProductInstanceByProductId(
productId
)
const checkResult = productInstance.checkAvailableDates(
productDefinition,
bookingStart
)
if (!checkResult) {
return false
}
michael-scheurer marked this conversation as resolved.
Show resolved Hide resolved
}
return true
}

/**
* LATEST BOOKING TIME CONSTRAINT
*/
Expand Down
46 changes: 45 additions & 1 deletion products/Product.js
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,8 @@ export default class Product {
filterProductDefinitionsByProductDefinitionAndAttributeKey(
attributeKey,
productDefinition,
excludeAttributeKeys = []
excludeAttributeKeys = [],
includeMauiOnly = false
) {
let filteredProductDefinitions = []
Copy link
Contributor

Choose a reason for hiding this comment

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

const


Expand Down Expand Up @@ -692,6 +693,12 @@ export default class Product {
filteredProductDefinitions.push(currentProductDefinition)
}

if (!includeMauiOnly) {
filteredProductDefinitions = filteredProductDefinitions.filter(
(prodDef) => !prodDef.isMauiOnly()
Comment on lines +697 to +698
Copy link
Contributor

Choose a reason for hiding this comment

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

do not modify existing objects. this obscures what the array contains. this should be named mauiOnlyProductDefinitions

)
}

// sort filtered product definitions
return filteredProductDefinitions.sort(
(productDefinitionA, productDefinitionB) => {
Expand Down Expand Up @@ -900,6 +907,43 @@ export default class Product {
return this.getAvailabilityDateRanges().getDateList(type)
}

/**
* Checks, if a product definition of this product has the full time span within validity dates and availability ranges
* @param productDefinitionInstance
* @param bookingStartInstance
* @return {boolean}
*/
checkAvailableDates(productDefinitionInstance, bookingStartInstance) {
// get an list of available dates of the product
const availableDateList = this.getAvailableDates('date')
// get a list of dates of the product definition (booking date + duration days)
const durationDays = productDefinitionInstance.getDurationDays()
const endDate = new Date(
bookingStartInstance.getFullYear(),
bookingStartInstance.getMonth(),
bookingStartInstance.getDate() + durationDays - 1,
0,
0,
0,
0
)
michael-scheurer marked this conversation as resolved.
Show resolved Hide resolved
const bookingDateList = DateHelper.getDateList(
bookingStartInstance,
endDate
)
for (let i = 0; i < bookingDateList.length; i++) {
const bookingDate = bookingDateList[i]
const foundAvailableDate = availableDateList.find((availableDate) => {
return availableDate.getTime() === bookingDate.getTime()
})
if (!foundAvailableDate) {
EventBus.$emit('notify', i18n.t('basket.dateNotAvailable'))
return false
}
}
Comment on lines +934 to +943
Copy link
Contributor

Choose a reason for hiding this comment

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

use for of. you are not using an index anywhere.

return true
}

getCurrentSeasonStart() {
const availableDates = this.getAvailableDates()
const todayMs = new Date().setHours(0, 0, 0, 0)
Expand Down