frx:skip_id : Adding the ability to have no id in frx:foreach elements [#2222743]

Currently if an element has the attribute frx:foreach="*" there's no way to ignore the id of this element. Forena will always assign this element an id.

For example:

   <root_tag frx:skip_root="true" id="forena-1" frx:block="dpisql/ACESHeaderDelivery">
      <Header frx:foreach="*" id="forena-2">
         <blah>stuff</blah>
      </Header>
    </root>

Header must always have an id in this situation.

I propose we create a frx:skip_id element, that when set to true, the frx:foreach element does not need an id. There are some cases in XML that's being validated where you cannot have an id for these frx:foreach elements.

So in the below example, the generated report would not display an id for the Header tag because we've added frx:foreach="true":

   <root_tag frx:skip_root="true" id="forena-1" frx:block="dpisql/ACESHeaderDelivery">
      <Header frx:foreach="*" frx:skip_id="true" id="forena-2">
         <blah>stuff</blah>
      </Header>
    </root>

Comment	File	Size	Author
#14	forena-no-ids.patch	3.22 KB	metzlerd
#14
#13	file-frx-version.png	180.35 KB	jamesdixon
#13	watchdog-has-forena-id-attr.png	34.63 KB	jamesdixon
#13	wysiwyg-frx-version.png	15.15 KB	jamesdixon
#9	no_forena_ids_2222743.patch	1.3 KB	metzlerd
#9
#2	frx_skip_id-2222743-2.patch	669 bytes	jamesdixon
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jamesdixon CreditAttribution: jamesdixon commented 21 March 2014 at 05:41

Version:

7.x-4.x-dev

» 7.x-3.x-dev

Comment #2

jamesdixon CreditAttribution: jamesdixon commented 21 March 2014 at 05:46

File	Size
frx_skip_id-2222743-2.patch	669 bytes

Here's a patch we used to do this on the 3.x branch.

Comment #3

jamesdixon CreditAttribution: jamesdixon commented 21 March 2014 at 05:47

Status:

Active

» Needs review

Comment #4

21 March 2014 at 14:25

Commit 59d09f4 on 7.x-3.x authored by jamesdixon:
```
#2222743 Skip Ids
```

Comment #5

metzlerd CreditAttribution: metzlerd commented 21 March 2014 at 14:33

Committed to 7.x-3.x-dev branch, but then began to wonder. The actual way to remove the attribute right now is to remove it from the source. It is true that forena adds ids to all elements that have frx attributes in order to give it a way to preserve the attrributes when a wysiwyg editor such as tinymce is used. Perhaps though it would be better to have the editor strip the forena-# tags off of the document before saving. In this fashion you could simply remove the ids from the .frx file an have the attributes not display. That is the way the current code would behave in both branches. That is if you manually remove the forena-1 id from this element, it will happily render without it and without this patch.

Does that make sense?

Comment #6

jamesdixon CreditAttribution: jamesdixon commented 21 March 2014 at 20:05

Yes that makes sense. Do you mean we should check for a frx:skip_id="true" on the element, and then if we are, do not create the id="forena-#" for that particular element?

Comment #7

jamesdixon CreditAttribution: jamesdixon commented 21 March 2014 at 20:06

Also, if we decide to go this route, which php file in forena is adding the id="forena-#"?

Thank you!

Comment #8

metzlerd CreditAttribution: metzlerd commented 22 March 2014 at 00:16

Not quite what I meant. What I'm thinking is that normally you can just remove the attribute altogether for all elements prior to saving. That is, if they don't mean anything then there is no real reason to save them. So if you don't want an id on an element you just leave the Id off. It's true that when you use the forena provided UI for editing a report it will add them, in the off chance that you are using the wysiwyg editor, we need the ids to put the frx attributes are put put back on the elements (The wysiwyg editors will generally strip them off).

Comment #9

metzlerd CreditAttribution: metzlerd commented 24 March 2014 at 16:40

File	Size
no_forena_ids_2222743.patch	1.3 KB

Here's a patch for the 7.4.x version that illustrates what I'm talking about. I'm leaving the 7.x-3.x branch the way that it is, but his should deal with the need for 7.x-4.x in a more robust way. That is you can just remove the ids when you don't want them to be included.

Comment #10

jamesdixon CreditAttribution: jamesdixon commented 10 April 2014 at 23:44

Yeah definitely that's a more intuitive way of doing it. I wasn't sure if there was another reason outside of the WYSIWYG editor validation the ids were being saved when none were specified.

Comment #11

metzlerd CreditAttribution: metzlerd commented 28 May 2014 at 20:59

Version:	7.x-3.x-dev	» 7.x-4.x-dev
Status:	Needs review	» Fixed

Given that this patch was committed to 7.x-4.x code tree (should be in as of beta2) going to call this one done.

Comment #12

jamesdixon CreditAttribution: jamesdixon commented 29 May 2014 at 00:08

Yes this works great thank you!

Comment #13

jamesdixon CreditAttribution: jamesdixon commented 29 May 2014 at 05:54

Status:

Fixed

» Needs work

File	Size
wysiwyg-frx-version.png	15.15 KB
watchdog-has-forena-id-attr.png	34.63 KB
file-frx-version.png	180.35 KB

Actually my bad, I missed the ids being added even though they're not in the saved .frx file.

When I save the frx report from WYSIWYG, the id="forena-[num]" appears in the editor, but when I check the actual myreport.frx file there is no id.

Running the report, for some reason it is reading the WYSIWYG version of the frx, not what's in the frx file. I tried clearing cache and it happened again.

Throwing a watchdog before we check for the id attribute in renderers/FrxRenderer shows that the forena-[num] id attribute is being included.

I added some screenshots to show what's happening. The Footer element is rendering with an id:

       <Footer id="1">
         <RecordCount>1</RecordCount>
       </Footer>

It's 1 instead of forena-6 as I have the numeric ids skininfo setting set to true.

Is it possible the WYSIWYG version of the .frx is being cached somehow? The frx version of the file clearly has no id, but the id is being picked up at render time somehow.