Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
FilterFormat file has @todo to rename the variable $format to $id and $name to $label to fit with other entities.
Proposed resolution
Do the renaming as said.
Remaining tasks
- rename
- RTBC the patch
- commit
User interface changes
None
API changes
None: those variables are private and should be accessed through getter/setters.
Data model changes
Beta phase evaluation
Issue category | Task because it is a small @todo to remove. |
---|---|
Issue priority | Normal because it is only about code cleaning. |
Unfrozen changes | Unfrozen because it only changes internal variables name. |
Comment | File | Size | Author |
---|---|---|---|
#51 | 2559377-interdiff-51-45.txt | 25.9 KB | jaxxed |
#51 | 2559377-51-core-filter-rename_format_and_name.patch | 25.9 KB | jaxxed |
#45 | core-rename_format_and_name_to_id_and_label-8-45.patch | 17.67 KB | martins.kajins |
#26 | 2559377-26.patch | 1.35 KB | singh_haneet |
#26 | interdiff-2559377-21-26.txt | 1.93 KB | singh_haneet |
Comments
Comment #2
Dom. CreditAttribution: Dom. commentedAttaching patch, simply renamed $format to $id and $name to $label.
Comment #8
joshi.rohit100Comment #9
XanoThanks! RTBC under condition that the tests pass.
Comment #11
subhojit777Comment #12
subhojit777Looks like there are many dependent tests based on this. I think we should break it down into separate issue and make this as meta issue. This way the patch will be easier to review.
Comment #13
joshi.rohit100Comment #15
XanoI forgot to mention this earlier, but this change needs an upgrade path for sites that already have these configuration entities imported. During the update, use the configuration system to load the configuration (NOT the entity), change the keys, and save it again.
Comment #16
martins.kajins CreditAttribution: martins.kajins commentedRenamed
Comment #17
martins.kajins CreditAttribution: martins.kajins commentedComment #19
joshi.rohit100Blank patch :)
Comment #20
joshi.rohit100Blank patch :)
Comment #21
martins.kajins CreditAttribution: martins.kajins commentedSorry for posting empty patch previously.
Comment #24
martins.kajins CreditAttribution: martins.kajins commentedComment #25
martins.kajins CreditAttribution: martins.kajins commentedComment #26
singh_haneet CreditAttribution: singh_haneet as a volunteer commentedAttaching patch and interdiff.
Comment #27
joshi.rohit100What I seems in #26, patch and diffs got interchanged. Patch should be diff and diff should be patch. Right ?
Comment #28
singh_haneet CreditAttribution: singh_haneet as a volunteer commentedNo, the patch is only for "FilterFormat.php" file. Interdiff was created for #21 in which "filter.format.plain_text.yml" file was changed which has been reverted in this diff.
Comment #29
subhojit777Comment #30
subhojit777Comment #33
keopxComment #34
keopxComment #37
martins.kajins CreditAttribution: martins.kajins commentedChange $format to $id
change $name to $label
Comment #38
martins.kajins CreditAttribution: martins.kajins commentedComment #40
Dom. CreditAttribution: Dom. commentedIs the patch #37 tested or failing ? I don't really understand the status of this issue...
Comment #41
martins.kajins CreditAttribution: martins.kajins commentedChange $format to $id
Comment #45
martins.kajins CreditAttribution: martins.kajins commentedComment #48
DuaelFrReading the last patch I think that something went wrong between #13 and now.
I think we should restart from #13 and find a way to fix the related tests without getting out of scope.
Comment #49
claudiu.cristeaSorry, I don't think this is a good idea in this stage of releasing the RC. I would move it to 8.1.x.
Comment #50
DuaelFrAgreed.
Thank you all for your work on this issue. Let's reopen it in a few weeks :)
Comment #51
jaxxed CreditAttribution: jaxxed at Wunder commentedI made these patches last week, but did not get a chance to upload them. I know that this issue is postponed, but I wanted to get the changes into the issue body.
Changes:
- id/label changes in tests (config, install, actual test calls);
- id/label change in the core standard install profile;
- id/label change in module install config;
- id/label change in module schema;
- reverted some incorrect changes of $format to $id, in cases where the $format was actually an entity object.
All of these changes are necessary in order to make the change as filter formats are create using the incorrect keys in all of those settings.
WARNING: don't approve these patches without testing them. There was a case where an exception was thrown related to having both an id key, and an 'edit-form' link. If it occurs during testing, then simply disable that link in the entity definition (annotation.)
Comment #52
DuaelFrThat's better! Thank you @jaxxed!
I had a quick look at your patch and it looks quite good.
Some comments follows, for the next person that's going to work on this.
$id might be undefined according to the foreach().
Inconsistent. Either use $format_label and $format_id, or $label and $id.
Comment #53
jaxxed CreditAttribution: jaxxed at Wunder commentedIf someone looks at that last patch, this looks like a mistake:
EDIT: this is mentioned in the previous comment, tag you're it.