Skip to content

Conversation

@nilshoerrmann
Copy link

Symphony 2.4 compatibility.

@jurajkapsz
Copy link

Works great under Symphony 2.5.1 but not sure what @michael-e ment with https://github.com/hananils/default_event_values/commit/a6da18d3e7144acf7e21aa04eb22f6e0124dd2a7

@michael-e
Copy link
Contributor

The problem is that with Symphony 2.5.x (also with latest integration code), the Default Events part of the events page is above the name for new events (see screenshot). For existing events, however, it is below the name (which is the correct position).

screenshot

@jurajkapsz
Copy link

Ah, I see, thanks. Hmm, but I got it right, bellow the Essentials in both new and existing event views. Any more data I can supply?

@michael-e
Copy link
Contributor

What's your version of Default Events?

@jurajkapsz
Copy link

0.7.0 , that is I took these changed files from this pull request.

@michael-e
Copy link
Contributor

Aha, so maybe that fixes it? I will wait for @brendo to pull this PR.

@michael-e
Copy link
Contributor

Now that is strange. I checked it, and I already use the code in this PR. Nevertheless, I have this issue with new events.

@jurajkapsz
Copy link

Hm, it is. Well, to confirm, I really took only the files from the Files changed tab above. I simply clicked to "view" them, then "raw" view and then save. Haven't used the git way on this one.

@michael-e
Copy link
Contributor

Unless @nilshoerrmann has an idea, I should open a separate issue if the problem persists with the next release . The pull request here will be necessary/useful anyway, I think.

@jurajkapsz
Copy link

Sounds good.

@nilshoerrmann
Copy link
Author

Unless @nilshoerrmann has an idea …

My only idea is that another extension is manipulating the markup which results in a different element count (either on your install or on ours). My other idea is that this XMLElement stuff sucks – but that doesn't help with this problem ;)

@michael-e
Copy link
Contributor

You might take a look if you see a suspicious extension, it's the project we are working on together. :-)

@nilshoerrmann
Copy link
Author

Yeah, I noticed the screenshot ;)

@nilshoerrmann
Copy link
Author

Uhm, I've taken a look the related install:

  • If I open my local copy, everything displays fine.
  • If I open the remote copy, I get the same result as Michael.

That doesn't make much sense …

@michael-e
Copy link
Contributor

I should have removed the Domain from the URL… Done!

You are right, it doesn't make much sense. :-(

@nilshoerrmann
Copy link
Author

Things that come into my mind:

  • another extension is interfering (see above)
  • the different installs behave differently due to different PHP versions, e. g. one taking text nodes into account while the other does not.

But I have to admit that this XMLElement thing feels a lot like black magic.
I'm calling @brendo for help.

@nilshoerrmann
Copy link
Author

I found the reason:

The markup of the remote install contains a XSRF token in a hidden input field which is not present on my local install:

<input name="xsrf" value="ZZrICBLq8+sdfs/E0YwEa+M" type="hidden">

@michael-e
Copy link
Contributor

This is injected by Symphony if you switch on XSRF protection.

But why does it work then if you edit an existing event? Is the position of the hidden field different then?

@nilshoerrmann
Copy link
Author

I don't know. The extension structure is a bit strange.
Let's wait for Brendan to chime in. He'll certainly know how to fix this properly.

@michael-e
Copy link
Contributor

@brendo, can you please pull this? Although there is still a small glitch (see discussion above), it should make the extension work with Sym 2.6.

@DavidOliver
Copy link

@brendo, I'm also hoping to use Default Event Values with Sym 2.6, so if you could pull it (and release?) that would be great. Thanks! (And apologies if I should be speaking to others instead of you about updating this.)

brendo added a commit that referenced this pull request Mar 2, 2016
Update layout for Symphony 2.4+ compatibility
@brendo brendo merged commit bbd1413 into brendo:integration Mar 2, 2016
@brendo
Copy link
Owner

brendo commented Mar 2, 2016

Done @DavidOliver & @michael-e. Let me know how it goes and we'll see if anything new pops up.

I haven't the time just yet to test myself, or peek into the changing order depending on if XSRF is enabled or not, so hopefully someone can help :)

@michael-e
Copy link
Contributor

I have been working with the code of this PR for a long time now, and it basically works. Regarding the small glitch mentioned above (the position on the event page being different in new/edit context), I can not replicate this with Symphony 2.6.7. Now, the position is always above the "essentials" section of the page, no matter if the event is new or edited. So the default values section should be moved down a bit on the page. Should I send a PR for this?

@DavidOliver
Copy link

Thanks, chaps. Using @michael-e's new version, the location of the Default Values section:

enable_xsrf of no: under Filters;
enable_xsrf of yes: above Filters.

This is the case both when creating a new event and editing an existing one.

Edit: I haven't looked at the code yet, but would inserting the XSRF input at the end of the form instead of the beginning be an option?

@michael-e
Copy link
Contributor

Yeah, OK, hmmm. That's still not perfect. It would be cool to skip the XSRF node… I will take a look.

@michael-e
Copy link
Contributor

Unfortunately I can't seem to find a suitable method in the XMLElement class to do what we are after. One might think about moving the XSRF element further down in the Symphony core… but I don't see any solution in the extension. @brendo?

@brendo
Copy link
Owner

brendo commented May 10, 2016

Sorry, long lag on my behalf. There's no method that will do it magically, but you could inspect the element via getChild(1) and if it was element was not what was expected the insertChildAt method could use a different index.

@michael-e
Copy link
Contributor

Isn't that too much "hardcoded magic"? That would create a rather strong dependancy on the backend code. Shouldn't we rather live with the result of #16, as desrcibed by @DavidOliver?

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

Successfully merging this pull request may close these issues.

6 participants