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

BugFix: Load predefined attribute when creating new page by composer #4470

Closed
wants to merge 1 commit into from
Closed

BugFix: Load predefined attribute when creating new page by composer #4470

wants to merge 1 commit into from

Conversation

HamedDarragi
Copy link
Contributor

Predefined attributes in /dashboard/pages/types/attributes/{ptID} are not being loaded when we create a new page.

@Remo

* @param mixed $var
* @return boolean
*/
function is_foreachable($var)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this from the PR and open an issue to discuss whether we need this global function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sure, the most important is the fix.
the global function is not really important.

@KorvinSzanto
Copy link
Member

This pull looks to break tests for some reason, can you resolve those test issues and reopen?

@aembler
Copy link
Member

aembler commented Oct 10, 2016

This is already fixed in v8

Sent from my iPhone

On Oct 10, 2016, at 2:24 PM, Korvin Szanto notifications@github.com wrote:

This pull looks to break tests for some reason, can you resolve those test issues and reopen?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@HamedDarragi
Copy link
Contributor Author

@KorvinSzanto test issues come from previous commits, you can see the latest merged PR #4443

@aembler as the v8 is not ready for production we need to fix bugs for the v7 until you release the v8
Thanks

@Remo

@Remo
Copy link
Contributor

Remo commented Oct 11, 2016

@aembler I strongly support Hamed here. 5.8 isn't stable and won't be fully backwards compatible, having at least a stable 5.7 branch on github would help a lot. It would be nice to run 5.8 for all projects, but in the end I have to run a business those concrete5 updates aren't always as cheap as one would wish them to be.

@aembler
Copy link
Member

aembler commented Oct 11, 2016

Fine, but I'm not sure this solution as written doesn't have unintended consequences

@Remo
Copy link
Contributor

Remo commented Oct 13, 2016

@HamedDarragi maybe you want to have a look at older 5.7 version? I'm pretty sure this used to work not too long ago.

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.

None yet

4 participants