Conversation
…essage but we do not in fact validate the values of this field.
|
SonarCloud Quality Gate failed.
|
ngm
left a comment
ngm
left a comment
There was a problem hiding this comment.
Left a few comments just to double check on a few things that seem fairly recently added but now removed. (scopes and attributes - as the search term for their usage doesn't necessarily correspond to the actual method name..)
| $website = $fields[5]; | ||
| $phone = $fields[6]; | ||
| $networks = $fields[7]; | ||
| $description = $fields[8]; |
There was a problem hiding this comment.
Looking at the DB, I think we're missing:
- area
- postcode if present
There was a problem hiding this comment.
Plus a new 'telephone' field that we said we would create I believe.
EDIT: sorry, I can see that phone is there. But I don't think we actually set it on the group later.
There was a problem hiding this comment.
Postcode we didn't discuss in the doc. That is their CP field - will set.
I'm adding phone at the moment, but it's in a new branch for RES-1756 as this one has been released. That will be ready for review soon.
| $group->longitude = $lng; | ||
| $group->country = $country; | ||
| $group->website = $website; | ||
| $group->free_text = $description; |
There was a problem hiding this comment.
In the DB there's a 'shareable_code' field. Does this get set automatically? Or do we need to set it here?
There was a problem hiding this comment.
I think you're right that we need to set it. Added (in 1756).
| { | ||
| $input = $this->argument('input'); | ||
| $output = $this->argument('output'); | ||
| $networks = $this->option('networks'); |
There was a problem hiding this comment.
What's the format of this - comma separated list of networks ids?
|
|
||
| if ($organisateur) { | ||
| $description .= "<p>Organisateur: " . htmlspecialchars($organisateur) . "</p>"; | ||
| } |
There was a problem hiding this comment.
We also said we would add the Inscription field to the Description.
| $website, | ||
| $phone, | ||
| $networks, | ||
| $description, |
There was a problem hiding this comment.
We should also be passing 'Ville' as the 'Area' field, somewhere.
ngm
left a comment
ngm
left a comment
There was a problem hiding this comment.
Some changes needed for RES-1756.
2 participants
Uh oh!
There was an error while loading. Please reload this page.