Fixed Issue #44 #65
Fixed Issue #44 #65
Conversation
Fixed Issue UpCloudLtd#44
| } | ||
| foreach (Capsule::table('tblproductconfigoptionssub')->where('configid', $optionId)->get() as $id) { | ||
| $parts = explode("|", $id->optionname); | ||
| $fprice = $parts[0] * "2"; | ||
| $monthlys = $this->manager->CurrencyConvert('EUR', $currencyCode, $fprice); | ||
| Capsule::table('tblpricing')->updateOrInsert(['type' => 'configoptions', 'relid' => $id->id, 'monthly' => $monthlys['convertedAmount'] ],['currency' => $currencyId]); | ||
| $relId = Capsule::table('tblproductconfigoptionssub')->where('optionname', $mem.'|'.$mvals)->where('configid', $optionId)->first()->id; | ||
| $fprice = $mem * "2"; | ||
| $monthlys = $this->manager->CurrencyConvert('EUR', $currencyCode, $fprice); | ||
| Capsule::table('tblpricing')->updateOrInsert(['type' => 'configoptions', 'relid' => $relId, 'monthly' => $monthlys['convertedAmount'] ],['currency' => $currencyId]); |
There was a problem hiding this comment.
I'm not sure I read this correctly, or how it is related to issue #44. Could we clarify the purpose of this change?
There was a problem hiding this comment.
I do not think that I made any changes createRAMFields function, please check again.
| } | ||
| foreach (Capsule::table('tblproductconfigoptionssub')->where('configid', $optionId)->get() as $id) { | ||
| $parts = explode("|", $id->optionname); | ||
| $fprice = $parts[0] * "6"; | ||
| $monthlys = $this->manager->CurrencyConvert('EUR', $currencyCode, $fprice); | ||
| Capsule::table('tblpricing')->updateOrInsert(['type' => 'configoptions', 'relid' => $id->id, 'monthly' => $monthlys['convertedAmount'] ],['currency' => $currencyId]); | ||
| $relId = Capsule::table('tblproductconfigoptionssub')->where('optionname', $cpu.'|'.$cvals)->where('configid', $optionId)->first()->id; | ||
| $fprice = $cpu * "6"; | ||
| $monthlys = $this->manager->CurrencyConvert('EUR', $currencyCode, $fprice); | ||
| Capsule::table('tblpricing')->updateOrInsert(['type' => 'configoptions', 'relid' => $relId, 'monthly' => $monthlys['convertedAmount'] ],['currency' => $currencyId]); |
There was a problem hiding this comment.
Same as above comment for memory, but for CPU here.
There was a problem hiding this comment.
I do not think that I made any changes createCPUFields function, please check again.
| } | ||
| foreach (Capsule::table('tblproductconfigoptionssub')->where('configid', $optionId)->get() as $id) { | ||
| $parts = explode("|", $id->optionname); | ||
| $fprice = $parts[0] * "0.10"; | ||
| $monthlys = $this->manager->CurrencyConvert('EUR', $currencyCode, $fprice); | ||
| Capsule::table('tblpricing')->updateOrInsert(['type' => 'configoptions', 'relid' => $id->id, 'monthly' => $monthlys['convertedAmount'] ], ['currency' => $currencyId] ); | ||
| $relId = Capsule::table('tblproductconfigoptionssub')->where('optionname', $storage.'|'.$vals)->where('configid', $optionId)->first()->id; | ||
| $fprice = $storage * "0.10"; | ||
| $monthlys = $this->manager->CurrencyConvert('EUR', $currencyCode, $fprice); | ||
| Capsule::table('tblpricing')->updateOrInsert(['type' => 'configoptions', 'relid' => $relId, 'monthly' => $monthlys['convertedAmount'] ], ['currency' => $currencyId] ); |
There was a problem hiding this comment.
Same as above comment for memory, but for storage here.
There was a problem hiding this comment.
same as above for createStorageFields function
| { | ||
| $groupId = Capsule::table('tblproductconfiggroups')->where('name', 'Configurable options for UpCloud Global')->first()->id; | ||
| $groupId = Capsule::table('tblproductconfiggroups')->where('name', 'Configurable options for UpCloud Cloud Linux')->first()->id; |
There was a problem hiding this comment.
Does "Cloud Linux" here and below mean that it is related to Linux instances set up from cloud-init templates? If I understand correctly, would be good to clarify the intents of these option sets in their names, for example
Configurable options for UpCloud - native Windows template VPS
Configurable options for UpCloud - native Linux template VPS
Configurable options for UpCloud - cloud-init Linux template VPS
There was a problem hiding this comment.
The names it self says https://prnt.sc/p-h7WOcY5_MF where Cloud Linux meaning cloud-init images
| $mems["$i"] = "$i Gigabyte (GB)"; | ||
| $mems["$i"] = "$i Gigabyte [GB]"; |
There was a problem hiding this comment.
I wonder what is the rationale for this change, and its relation to #44?
There was a problem hiding this comment.
It was creating some conflicts so added [] brackets
| if ($vmtype == 'nativewindows' && preg_match('/Windows/', $template['title'])) { | ||
| $templateIds[$template['uuid']] = $template['title']; | ||
| } elseif ($vmtype == 'nativelinux' && !preg_match('/Windows/', $template['title'])) { | ||
| $templateIds[$template['uuid']] = $template['title']; | ||
| } | ||
| } elseif ($template['template_type'] == "cloud-init" && $vmtype == 'cloudinit') { | ||
| $templateIds[$template['uuid']] = $template['title']; |
There was a problem hiding this comment.
The bodies of all these conditions are the same,
$templateIds[$template['uuid']] = $template['title'];
Is this on purpose? If yes, I feel this would be clearer refactored into a single if statement with a series of || separated conditions, instead of a nested if-else chain. As currently written, it gives the feeling that there might be a bug in that all the condition bodies are the same (i.e. maybe they should be somehow different)?
There was a problem hiding this comment.
How they are same? Have you checked the conditions?
| } | ||
| if (!$vmtype) { | ||
| foreach ($templates as $template) { | ||
| $templateIds[$template['uuid']] = $template['title']; |
There was a problem hiding this comment.
Here's yet another same condition body as above.
There was a problem hiding this comment.
Do verify the conditions for the function getTemplateIds and let me know your issues
Fixed Issue #44