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

[4.0] New banner fails to save #23073

Closed
wants to merge 599 commits into from
Closed

[4.0] New banner fails to save #23073

wants to merge 599 commits into from

Conversation

phalouvas
Copy link
Contributor

Pull Request for Issue #23072

Summary of Changes

The problem is on file /administrator/components/com_banners/Table/BannerTable
line 204. Change from:
$client = Table::getInstance('Client', NAMESPACE . '', array('dbo' => $db));
to
$client = new ClientTable($db);

patch.txt

Testing Instructions

Create new banner
Put a title
Choose a client.
Save.

Expected result

To save a new banner on the selected client.

Actual result

An error has occurred.
0 Call to a member function load() on boolean

Documentation Changes Required

brianteeman and others added 30 commits October 15, 2018 01:12
* [4.0] configuration tab background

Small tweak to the background colour of the active color to increase the contrast between active and non-active tabs

* Revert "[4.0] configuration tab background"

This reverts commit 79c640e.

Do the work in the variaables.scss

* make the change in a variable

* oops
after jquery migrate version change
* Fix jQuery Minicolors positioning

* Hound
* Update dependencies

* Use identical operators for plugins

* Update calendar.php

* Update color.php

* Update editor.php

* Update imagelist.php

* Update integer.php

* Update list.php

* Update radio.php

* Update url.php

* Update usergrouplist.php

* Update user.php

* Update textarea.php

* Update text.php

* Update sql.php

* Update media.php

* Update blog.php

* Update imagelist.php
PR for #22561

A recent PR was merged to ensure that the postinstallation messages dropdown is not hidden behind the toolbar. However it wasn't quite correct and the user menu was still hidden

With this PR (not testable with patchtester) both the user menu and the postintallation messages haave the correct z-index
* [4.0] Spacer text

Two examples to test
1. create a menu item "Featured contents", and go to 2nd TAB "List layouts"
2. com_media options

* hound

* fonts

* oops

* Revert "oops"

This reverts commit a27b74c.

* Revert "fonts"

This reverts commit ed6bf0e.

* Update _form.scss
Phil Taylor and others added 6 commits November 12, 2018 09:53
* Fix JS alerts for Joomla/Extension update check

Closes #22971
Related #22981

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>

* codestyle

* Codestyle

* Improve documentation of function

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>

* Add message type `warning` and downgrade message from error to a warning

* add warning example to docblock
@richard67
Copy link
Member

Obviously you made this PR against the wrong branch: It should be 4.0-dev, but you made it for staging (which is 3.9)

@phalouvas
Copy link
Contributor Author

      Obviously you made this PR against the wrong branch: It should be 4.0-dev, but you made it for staging (which is 3.9)

Ooops. Can I change it, or need to create new?

@SharkyKZ
Copy link
Contributor

You can change it. Click Edit button to the right of the title.

@ggppdk
Copy link
Contributor

ggppdk commented Nov 14, 2018

Ooops. Can I change it, or need to create new?

When you click edit button it will allow you edit the title of this PR but also you can change the branch

https://help.github.com/articles/changing-the-base-branch-of-a-pull-request/

@phalouvas phalouvas changed the base branch from staging to l10n_4.0-dev November 14, 2018 07:06
@phalouvas
Copy link
Contributor Author

Thanks. I just did.

@SharkyKZ
Copy link
Contributor

Change it to 4.0-dev, not l10n_4.0-dev.

@phalouvas
Copy link
Contributor Author

There is no option for 4.0-dev

@laoneo
Copy link
Member

laoneo commented Nov 14, 2018

I guess you somehow screwed up this pr. Probably the best option would be to create a new one.

@wilsonge
Copy link
Contributor

You're trying to merge 4.0 from the CMS into another repo. I assume you meant to do a PR from your own fork

@wilsonge
Copy link
Contributor

I've manually changed this with 329fd3b

@wilsonge wilsonge reopened this Nov 14, 2018
@wilsonge wilsonge closed this Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet