BelgiumPostCode form type #196
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR adds a new Symfony form type for selecting Belgian postcodes and municipalities by introducing a value object, model transformer, Intl ResourceBundle loader, and raw dataset, along with service registration, updated dependencies, and relevant translations. Entity relationship diagram for BelgiumPostCode value objecterDiagram
BELGIUM_POST_CODE {
string postcode
string municipality
}
Class diagram for new BelgiumPostCode form type and related classesclassDiagram
class BelgiumPostCodeType {
+buildForm(FormBuilderInterface, array): void
+configureOptions(OptionsResolver): void
+getParent(): ?string
+getBlockPrefix(): string
}
class BelgiumPostCodes {
+static getNames(): array<string, string>
#static getPath(): string
}
class BelgiumPostCode {
+string postcode
+string municipality
+__construct(string postcode, string municipality)
}
BelgiumPostCodeType --> BelgiumPostCode : uses
BelgiumPostCodeType --> BelgiumPostCodes : uses
BelgiumPostCodes --|> ResourceBundle
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @absumo - I've reviewed your changes - here's some feedback:
- In services.php, you're using the same service id (
framework.file_type) for both FileType and BelgiumPostCodeType—use a unique id for the new form type. - BelgiumPostCodes::getNames currently hard-codes the locale to 'nl'; consider using the application’s current locale (Intl::getLocale() or Symfony’s locale) to support multiple languages.
- The generated data file for postcodes is extremely large and loaded in memory all at once; consider moving to a more efficient lazy-loaded or external resource to avoid potential performance and repository bloat.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In services.php, you're using the same service id (`framework.file_type`) for both FileType and BelgiumPostCodeType—use a unique id for the new form type.
- BelgiumPostCodes::getNames currently hard-codes the locale to 'nl'; consider using the application’s current locale (Intl::getLocale() or Symfony’s locale) to support multiple languages.
- The generated data file for postcodes is extremely large and loaded in memory all at once; consider moving to a more efficient lazy-loaded or external resource to avoid potential performance and repository bloat.
## Individual Comments
### Comment 1
<location> `src/Form/Type/BelgiumPostCodeType.php:57` </location>
<code_context>
+ 'choice_translation_domain' => false,
+ 'invalid_message' => 'Please select a valid postcode.',
+ 'placeholder' => '',
+ 'autocomplete' => true,
+ 'label' => 'Postcode and municipality',
+ ]);
</code_context>
<issue_to_address>
The 'autocomplete' option is not a standard FormType option.
To set an HTML autocomplete attribute, use the 'attr' option: ['attr' => ['autocomplete' => 'on']]. The current 'autocomplete' option will be ignored by Symfony forms.
</issue_to_address>
### Comment 2
<location> `src/Intl/BelgiumPostCodes.php:14` </location>
<code_context>
+ */
+ public static function getNames(): array
+ {
+ return self::readEntry(['Names'], 'nl', false);
+ }
+
</code_context>
<issue_to_address>
Hardcoding 'nl' locale limits flexibility.
Consider making the locale parameter configurable or using the current request locale to support multiple languages.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
/**
* @return array<string, string>
*/
public static function getNames(): array
{
return self::readEntry(['Names'], 'nl', false);
}
=======
/**
* @param string|null $locale
* @return array<string, string>
*/
public static function getNames(?string $locale = null): array
{
if ($locale === null) {
$locale = \Locale::getDefault();
}
return self::readEntry(['Names'], $locale, false);
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/Intl/Resources/data/belgiumpostcodes/nl.php:18` </location>
<code_context>
+ 'Names' => [
+ "0612Sinterklaas" => "0612 - Sinterklaas",
+ "1000Brussel" => "1000 - Brussel",
+ "1005Verenigde Vergadering van de Gemeenschappelijke " => "1005 - Verenigde Vergadering van de Gemeenschappelijke ",
+ "1006Raad van de Vlaamse Gemeenschapscommissie" => "1006 - Raad van de Vlaamse Gemeenschapscommissie",
+ "1007Assemblée de la Commission Communautaire Française" => "1007 - Assemblée de la Commission Communautaire Française",
</code_context>
<issue_to_address>
Trailing space in key and value may cause lookup inconsistencies.
Please check if the trailing spaces in both the key and value are intentional and match the data source, as they may cause subtle lookup bugs.
</issue_to_address>
### Comment 4
<location> `src/Intl/Resources/data/belgiumpostcodes/nl.php:1951` </location>
<code_context>
+ "7090Braine-le-Comte" => "7090 - Braine-le-Comte",
+ "7090Hennuyères" => "7090 - Hennuyères",
+ "7090Henripont" => "7090 - Henripont",
+ "7090Petit-R�ulx-lez-Braine" => "7090 - Petit-R�ulx-lez-Braine",
+ "7090Ronquières" => "7090 - Ronquières",
+ "7090Steenkerque" => "7090 - Steenkerque",
</code_context>
<issue_to_address>
Non-ASCII character encoding issue in 'Petit-R�ulx-lez-Braine'.
The character '�' in this entry may cause lookup or display problems. Please check and fix the encoding here and in similar cases.
</issue_to_address>
### Comment 5
<location> `src/Intl/Resources/data/belgiumpostcodes/nl.php:1997` </location>
<code_context>
+ "7181Arquennes" => "7181 - Arquennes",
+ "7181Familleureux" => "7181 - Familleureux",
+ "7181Feluy" => "7181 - Feluy",
+ "7181Petit-R�ulx-lez-Nivelles" => "7181 - Petit-R�ulx-lez-Nivelles",
+ "7190Écaussinnes-d'Enghien" => "7190 - Écaussinnes-d'Enghien",
+ "7190Marche-lez-Écaussinnes" => "7190 - Marche-lez-Écaussinnes",
</code_context>
<issue_to_address>
Encoding issue in 'Petit-R�ulx-lez-Nivelles'.
Please correct the encoding for 'Petit-R�ulx-lez-Nivelles' to maintain data integrity.
</issue_to_address>
### Comment 6
<location> `src/Intl/Resources/data/belgiumpostcodes/nl.php:2021` </location>
<code_context>
+ "7340Wasmes" => "7340 - Wasmes",
+ "7350Hainin" => "7350 - Hainin",
+ "7350Hensies" => "7350 - Hensies",
+ "7350Montr�ul-sur-Haine" => "7350 - Montr�ul-sur-Haine",
+ "7350Thulin" => "7350 - Thulin",
+ "7370Blaugies" => "7370 - Blaugies",
</code_context>
<issue_to_address>
Encoding issue in 'Montr�ul-sur-Haine'.
The name includes a non-standard character ('�'); please check the encoding and correct it.
</issue_to_address>
### Comment 7
<location> `src/Intl/Resources/data/belgiumpostcodes/nl.php:2205` </location>
<code_context>
+ "7911Frasnes-lez-Buissenal" => "7911 - Frasnes-lez-Buissenal",
+ "7911Hacquegnies" => "7911 - Hacquegnies",
+ "7911Herquegies" => "7911 - Herquegies",
+ "7911Montr�ul-au-Bois" => "7911 - Montr�ul-au-Bois",
+ "7911Moustier" => "7911 - Moustier",
+ "7911Oeudeghien" => "7911 - Oeudeghien",
</code_context>
<issue_to_address>
Encoding issue in 'Montr�ul-au-Bois'.
Please correct the encoding of 'Montr�ul-au-Bois' to prevent lookup or display problems.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
absumo
force-pushed
the
39-autocomplete-postcodes
branch
from
d88584d to
b66fb14
Compare
tijsverkoyen
approved these changes
Aug 11, 2025
tijsverkoyen
requested changes
Aug 11, 2025
tijsverkoyen
approved these changes
Aug 11, 2025
|
@absumo schrijf misschien nog docs. En dan mag je mergen. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add a new form type for selecting Belgian postcodes with municipality names, backed by a generated dataset and Symfony Intl.
New Features:
Build:
Documentation: