OK, so...I'll begin by admitting that this patch significantly changes the CER code base in the hope of fixing (or at least mitigating) a number of outstanding issues in the queue. It is an in-depth refactoring of CER's core code.

In a nutshell, the patch replaces cer.crud.inc - which is filled with complex, hard-to-parse, and nearly-impossible-to-extend logic derived almost verbatim from CNR - with handler.inc, which implements the basic CER logic as an extendable plugin-like object. In doing this, I think I may also have solved, or at least improved, a few long-standing issues with CER.

My hope is that this approach will:

  • Ease the port to Drupal 8's object-oriented architecture
  • Ensure that the code can be more easily maintained (and patched) in the future by other developers
  • Make the module more flexible for those who may need more advanced handling of corresponding references (an idea I got while working on http://drupal.org/node/1961026)

I've taken care to make sure that this code, like cer.crud.inc before it, passes the automated test suite (included with the patch). Everything in handler.inc is also documented so as to be readily understandable by other coders. If anyone would like to give it a look and a try, I'd appreciate any and all feedback.

I still need to do more testing, but here are some of the issues I'm hoping this admittedly drastic change will solve, or at least bring that much closer to "fixed":

CommentFileSizeAuthor
#1 cer-handler-refactor.patch33.88 KBphenaproxima
cer-handler-refactor.patch33.29 KBphenaproxima

Comments

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new cer-handler-refactor.patch33.88 KB

Updated patch attached - fixed a few comments in handler.inc and threw exceptions in the case of "reference overloading" (a fancy term for "putting too many values into a field").

DerTobi75’s picture

DerTobi75 commented

Great news, any chance this also fixes this issue: http://drupal.org/node/1729666 ?

Regards,

Tobi

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

@ DerTobi75: No, it will not address that issue - that's a problem with the admin screen's code, which this patch doesn't touch.

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
commented

@phenaproxima i would suggest that you create a 7.x-2.x branch starting with this patch. It is a major re-write, and fixes a lot of issues. After the new branch is created and has a dev version, just deprecate 7.x-1.x

Just my $0.02.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: New OO architecture, incl. several fixes » New OO architecture, incl. several fixes (7.x-2.x branch)

@chertzog, that sounds like a great idea to me. It would be a graceful way to fold current patches into 1.x while providing a clear way forward. Not only that, but (at least for now) 7.x-2.x could be a drop-in replacement for 7.x-1.x, since it doesn't involve any special changes to the database or anything. I will create the 2.x branch tonight.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Closed (fixed)

Created the 7.x-2.x dev branch just now from the latest 1.x snapshot, plus this patch and fixes for other issues. I also created a dev release for 7.x-2.x, but it's still unpublished. Do new dev releases need to be approved by someone before they appear as a download?

Closing this issue now - onward and upward!

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes

Minor grammatical fixes