backdrop-contrib / paragraphs

Paragraphs module to control your content flow
https://backdropcms.org/project/paragraphs
GNU General Public License v2.0
5 stars 11 forks source link

Add dependency on Entity Plus #48

Closed laryn closed 4 years ago

laryn commented 4 years ago

Removing the EMW dependency ended up causing a bunch of loose ends that nobody seems to have time to comb through. I'm planning to add a dependency on Entity Plus at this point which seems to solve a handful of issues in the queue and will make things simpler for me.

The biggest issue will be figuring out how to introduce a dependency when many people already have the module installed. How can we make sure they install the latest version of Entity Plus before upgrading Paragraphs?

docwilmot commented 4 years ago

Would be good to summarize the loose ends to see if we can work on keeping this light, without EP getting involved. I'm willing to help get that done in the next few days.

laryn commented 4 years ago

@docwilmot These "has PR" issues are all solved if I add Entity Plus (currently the entityplus branch of Paragraphs):

I'm not sure what it would take to rework this module to solve all of those without it.

laryn commented 4 years ago

So in early testing, it looks like upgrading Paragraphs to a version that is dependent on Entity Plus (if you don't have Entity Plus installed) crashes any page that has a Paragraph on it (as expected: Error: Class 'ParagraphsItemController' not found). The admin pages still work, and Entity Plus can be installed/enabled through the UI if you navigate to the admin page after the fatal error. Not ideal, of course, but recoverable.

It would be a good idea if we go this route to add a red error button to the admin toolbar if Paragraphs is installed but Entity Plus is not, to make sure it is clear what the issue is. And if you do the upgrade through the UI, you should see the red warning button immediately.

herbdool commented 4 years ago

Maybe some warning can be added to the install file. It might help people find out what is wrong.

klonos commented 4 years ago

Actually this would be a great opportunity to test how the Project Installer/Updater behaves when dependencies are introduced (if it is decided to introduce the dependency after all that is).

...I would expect the update to show as available, but denoting that there is a missing dependency. When the update proceeds, I would expect any dependencies to be downloaded and installed automatically.

laryn commented 4 years ago

So I tested as follows:

On the update step it halted and indicated dependencies weren't met: Screen Shot 2020-03-05 at 11 36 37 PM

So this would work similarly, I think. I also added (in the entityplus branch) a red warning flag in the admin toolbar/status page if Entity Plus was not installed.

herbdool commented 4 years ago

That's a smart approach. Looks like it works then.

laryn commented 4 years ago

It works, kind of. Ideally the requirements check would happen before it replaces code (right now it happens after it replaces code but before it tries any potential database updates). But this may be the best we can do.

...I would expect the update to show as available, but denoting that there is a missing dependency. When the update proceeds, I would expect any dependencies to be downloaded and installed automatically.

@klonos Yes, ☝️ would be ideal!