Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating Symphony #1618

Closed
2 of 4 tasks
nilshoerrmann opened this issue Jan 22, 2013 · 19 comments
Closed
2 of 4 tasks

Updating Symphony #1618

nilshoerrmann opened this issue Jan 22, 2013 · 19 comments

Comments

@nilshoerrmann
Copy link
Contributor

I know this is certainly not the most common task, but I recently had to update an install from Symphony 2.0.6 to the latest version. Following @brendo's advice, I updated step by step: from 2.0 to 2.1 to 2.2 to 2.3. I ran (and am still running) into some problems. Most of them are related to extensions that don't update properly (especially changed database columns are an issue) but I have some issues with the core as well.

First and foremost, Data Sources are a problem:

  • In Symphony 2.0 there were no formatted and unformatted pseudo elements. Given an input field text the old Data Sources will reference this field as text and not as text:formatted or text:unformatted. Nevertheless, Symphony 2.3 still recognise the field and treats it as an unformatted value, wrapping it in <![CDATA[]]> (strangely also adding a <p /> element if the content contains HTML, e. g. <![CDATA[<p><p>original paragraph 1.</p><p>original paragraph 2.</p></p>]]>). The problems begin when you edit a Data Source: the field will not be selected in the interface because the UI is only aware of text:formatted and text:unformatted and not of text. As soon as you save your Data Source, your field is removed. I was quite irritated because this broke my frontend and I didn't understand why. If Symphony treats text as text:unformatted on the frontend, it should also select this field value in the DS editor.
  • In Symphony versions prior 2.3 there were only single output parameters. While Symphony recognises single value settings, it is strange that (of course) the name in the interface is changed from text to $ds-my-datasource.text although $ds-my-datasource is still available in the XML for compatibility reasons. In my eyes, Symphony should flag this problem in a system message when loading an old Data Source with a single value (not an array as in Symphony 2.3) so that the developer knows that he has to adjust his templates in order to keep them future proof.
  • Static XML Data Sources will display without any content in the Data Source editor.
  • Dynamic XML Data Sources will display without any settings in the Data Source editor.

There are also some issues with changed filtering syntax that you don't recognise immediately (the date field has been improved quite a bit and other fields as well). The user is not aware that this might be a problem – and that's a problem.

Symphony doesn't highlight old Data Source files in the backend at all so you are not aware at all that you need to take special care here. In contrary, Symphony behaves as if it silently updated everything in the background – which is not true as you see above. Old Data Sources should be flagged somehow, maybe in a special column in the index view and as a system message in the DS editor.

There also seem to be some problems with events that I haven't managed to debug yet. I'm not sure if something changed in the execution order of Data Sources and events – at least I'm seeing some strange behaviours (I have to deal with a custom shopping cart that has gone mad after upgrading).

@nilshoerrmann
Copy link
Contributor Author

PS: By the way, it's quite irritating that Symphony defaults to :unformatted output if no format is available in the Data Source. In Symphony 2.0, only formatted output existed, the new feature introduced by either Symphony 2.0.8 or 2.1 was the unformatted option. So it would be logical to default to :formatted in order to keep sites up and running (if – for instance – you are using the ninja technique, you site will break because formatted content is expected on not unformatted text wrapped in CDATA).

@brendo
Copy link
Member

brendo commented Jan 22, 2013

@allen or @rowan-lewis, why did we default to :unformatted if :formatted was the default?

In my eyes, Symphony should flag this problem in a system message when loading an old Data Source with a single value (not an array as in Symphony 2.3) so that the developer knows that he has to adjust his templates in order to keep them future proof.

This is definitely possible. Since Symphony 2.2.x we changed the DSE/EE so that the resulting file is saved with the version of Symphony that generated the file. Logic can be added that either updates these files during migration, or displays an Alert in the backend.

From memory there are a couple of times where there have been changes to Datasources:

  • The introduction of modes (:formatted/:unformatted)
  • Creating a setting to not fetch associated entry counts (huge performance boost)
  • Wrapping the execution function in try/catch logic to handle 404's
  • The renaming of grab to execute
  • Handling single and multiple output parameters

Because there is a number of things to consider, I think they should be in the update files, not in the actual core logic (we should be trying to strip the legacy code out, not add to it). This will be a little hard to do, as Symphony 2.3 only officially supports updating from a 2.2.x site. Some of these old changes could be added to the 2.2.0 migration to handle the earlier versions.

Static XML Data Sources will display without any content in the Data Source editor.
Dynamic XML Data Sources will display without any settings in the Data Source editor.

Anymore information on this? I can't recall anything specifically that may of affected these in particular.

@nilshoerrmann
Copy link
Contributor Author

Logic can be added that either updates these files during migration, or displays an Alert in the backend.

We shouldn't update automatically. Due to the interaction of Data Sources with fields, too many things that the core is not aware of might need to be adjusted. We should just properly flag outdated Data Sources and maybe link to an external guide that highlights the most obvious changes needed.

I'm not sure if this is possible, but it would be great if fields could also add special flags. Think of the improved filter logic of the date field: if the core provided the field with the information that an 2.0 Data Source is used in Symphony 2.3, the field could add information to the backend, e. g. by adding system messages or additional texts in the filtering interface.

Anymore information on this?

Not really. No related errors in the logs. Just disappearing data in the interface:

  • If I load the Data Source, the data is not displayed. I need to check if the data is lost after saving.
  • If I load the Data Source, and enter the original data again and hit save, the data is correctly stored in the updated file but still doesn't display in the interface.

@nilshoerrmann
Copy link
Contributor Author

By the way, there is another issue with older Data Sources:

Symphony Warning: Missing argument 1 for datasourceteilnehmen_monate::grab(), called in /symphony/content/content.blueprintsdatasources.php on line 166 and defined

An error occurred in /Users/nilshoerrmann/Sites/Webseiten/stil-bluete.net/workspace/data-sources/data.teilnehmen_monate.php around line 32

It's this line:

public function grab(&$param_pool){

@brendo
Copy link
Member

brendo commented Jan 22, 2013

that an 2.0 Data Source is used in Symphony 2.3

One problem with this is that we don't know that it's a 2.0 Datasource. Prior to the Symphony version being saved all Datasources were saved with a version of 1.

If I load the Data Source, the data is not displayed. I need to check if the data is lost after saving. If I load the Data Source, and enter the original data again and hit save, the data is correctly stored in the updated file but still doesn't display in the interface.

Odd, would you be able to put this in a private gist and DM or email me them for debugging?

By the way, there is another issue with older Data Sources:

This is not possible to correct without modifying the datasource file on update, it's a PHP warning because the Data Source signatures have changed, not a Symphony error as such.

@brendo
Copy link
Member

brendo commented Jan 22, 2013

Did you encounter #1605 by any chance?

@nilshoerrmann
Copy link
Contributor Author

This is not possible to correct without modifying the datasource file on update, it's a PHP warning because the Data Source signatures have changed, not a Symphony error as such.

Okay, to correct my comment above: this is something we can and should correct on update in my eyes. But we should still flag the Data Source as outdated.

One problem with this is that we don't know that it's a 2.0 Datasource.

It would still be a good start to provide the information that the Data Source is from an old version. So Symphony 2.3 should be able to say < 2.3 or == 2.3 and Symphony 2.4 should be able to say < 2.3 or == 2.3 or == 2.4.

@nilshoerrmann
Copy link
Contributor Author

Did you encounter #1605 by any chance?

With regards to which issue?

@nilshoerrmann
Copy link
Contributor Author

To be more precise regarding the updating:

We should adjust the specific functions (I think removing the & does the trick) but we shouldn't update to the new Data Source classes. This should only happen when I resave the Data Source.

@nilshoerrmann
Copy link
Contributor Author

Odd, would you be able to put this in a private gist and DM or email me them for debugging?

I just sent you two Data Sources, a static and a dynamic one.

@brendo
Copy link
Member

brendo commented Jan 22, 2013

It would still be a good start to provide the information that the Data Source is from an old version.

We have started doing this since about 2.2 IIRC, so we can determine in future.

With regards to which issue?

Your password field not being updated to 150 characters to store your mitigated password using the new logic.

I just sent you two Data Sources, a static and a dynamic one.

Thanks, I found something about Static DataSources in the commit history

@nilshoerrmann
Copy link
Contributor Author

Your password field not being updated to 150 characters to store your mitigated password using the new logic.

The update log contained no errors.

@nilshoerrmann
Copy link
Contributor Author

Thanks, I found something about Static DataSources in the commit history

Actually, I don't understand what's happening in that commit. Is it logical that no content is displayed?

I cannot resave the Data Source without copy and pasting the original static XML and although the file is updated when I do so, the editor still displays nothing.

It's heavily escaping my input. This …

<teilnehmen-monate>
    <monat handle="januar" nummer="01">Januar</monat>
    <monat handle="februar" nummer="02">Februar</monat>
    <monat handle="maerz" nummer="03">März</monat>
    <monat handle="april" nummer="04">April</monat>
    <monat handle="mai" nummer="05">Mai</monat>
    <monat handle="juni" nummer="06">Juni</monat>
    <monat handle="juli" nummer="07">Juli</monat>
    <monat handle="august" nummer="08">August</monat>
    <monat handle="september" nummer="09">September</monat>
    <monat handle="oktober" nummer="10">Oktober</monat>
    <monat handle="november" nummer="11">November</monat>
    <monat handle="dezember" nummer="12">Dezember</monat>
</teilnehmen-monate>

… becomes this:

public $dsParamSTATIC = '<teilnehmen-monate>
    <monat handle=\\\"januar\\\" nummer=\\\"01\\\">Januar</monat>
    <monat handle=\\\"februar\\\" nummer=\\\"02\\\">Februar</monat>
    <monat handle=\\\"maerz\\\" nummer=\\\"03\\\">März</monat>
    <monat handle=\\\"april\\\" nummer=\\\"04\\\">April</monat>
    <monat handle=\\\"mai\\\" nummer=\\\"05\\\">Mai</monat>
    <monat handle=\\\"juni\\\" nummer=\\\"06\\\">Juni</monat>
    <monat handle=\\\"juli\\\" nummer=\\\"07\\\">Juli</monat>
    <monat handle=\\\"august\\\" nummer=\\\"08\\\">August</monat>
    <monat handle=\\\"september\\\" nummer=\\\"09\\\">September</monat>
    <monat handle=\\\"oktober\\\" nummer=\\\"10\\\">Oktober</monat>
    <monat handle=\\\"november\\\" nummer=\\\"11\\\">November</monat>
    <monat handle=\\\"dezember\\\" nummer=\\\"12\\\">Dezember</monat>
</teilnehmen-monate>';

@brendo
Copy link
Member

brendo commented Jan 22, 2013

I just fixed that and restored some broken functionality for handling Symphony 2.2, 2.2.2 - 2.2.5 and then 2.3+ static DS's.

@brendo
Copy link
Member

brendo commented Jan 22, 2013

And just pushed another commit. There was bad regression in the integration branch introduced when the notice prevention commits were merged. This has now been corrected!

@nilshoerrmann
Copy link
Contributor Author

Great, static and dynamic Data Sources work perfectly again. Thanks!

@brendo
Copy link
Member

brendo commented Feb 15, 2013

This Data Source is using the old parameter syntax, eg. $ds-name. To ensure future compatibility, the syntax should be altered, eg. $ds-name.field-handle.

How's that copy for detecting an old style parameter? Currently I'm searching the parameter output, required param setting and the filters for anything that starts with $ds- and doesn't contain a ..

@nilshoerrmann
Copy link
Contributor Author

Yes, old parameters will not contains dots: it's always $ds- plus the handle of the Data Source.
The new syntax added the field names.

@allen
Copy link
Contributor

allen commented May 10, 2013

@allen or @rowan-lewis, why did we default to :unformatted if :formatted was the default?

The reason was that unformatted data wrapped in CDATA is safer to output. Given that there is no text formatter chosen, we cannot assume that the source data would be valid or well-formed XML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants