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

Implements accessing the UUIDs of a config items by tag reference #2665

Closed
wants to merge 5 commits into from

Conversation

ndejong
Copy link
Contributor

@ndejong ndejong commented Aug 22, 2018

Raised as issue #2664
Discussed in forum here

@ndejong
Copy link
Contributor Author

ndejong commented Aug 22, 2018

Even tried creating a new branch to prevent those previous commits getting tangled up with this PR here - oh, git...

for uuid in self._template_in_data['__uuid__']:
if self._template_in_data['__uuid__'][uuid] == item:
uuids.append(uuid)
return uuids
Copy link
Member

Choose a reason for hiding this comment

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

should be done for a single element

@ndejong
Copy link
Contributor Author

ndejong commented Aug 23, 2018

This updated diff makes it more explicit about there the uuid string is coming from - also adds a break once the uuid is found since we don't need to keep searching and can skip to the next

@AdSchellevis
Copy link
Member

@ndejong if we add the attributes to the object, would that be sufficient for you? I'm not sure we should add an extractor for all uuids if most use-cases need the uuid of the item traversing.

@ndejong
Copy link
Contributor Author

ndejong commented Aug 23, 2018

@AdSchellevis - I'm probably happy with any solution that is clear and easy to use.

If step back for a moment and consider why I wrote an extractor - As a relative newbie in the OPNsense codebase I felt like I wanted a solution that made the templates easier to write and understand. To get there I first dug through other templates and then through code to discover the data-structures inside the template helpers at which point it did not occur to me that I could simply use the item object itself as the comparator as @fabianfrz pointed out in the Nginx work around - my extractor uses the same approach.

The idea of injecting a special __uuid__ attribute into each item certainly works, perhaps the comment I'd make is that, again as a newbie it took me until I discovered where the template helper was and then finding a way to observe the data-structures inside that the __uuid_tags__ and __uuid__ elements became apparent - it was all a bit magic :) and so I wonder if the per item __uuid__ attribute will be too much magic and/or get confused with the existing __uuid__ element that feels like it could have been called __uuids__ (with the "s")

I'm perhaps biased here, but I feel like the explicit function that does what it says getUUIDs() makes it easier for the next plugin developer that comes stumbling along.

Totally your call.

@fabianfrz
Copy link
Member

@AdSchellevis the uuid as a property would be exactly what I need. be careful because you may overwrite data. In PHP i get it as [(at) attibutes]["uuid"]. The advantage is, that an @ cannot be a valid XML tag.

@AdSchellevis
Copy link
Member

@fabianfrz I know, As soon as I have a proper fix, I'll post it in the issue #2664 and close this one when done.

@AdSchellevis
Copy link
Member

@ndejong @fabianfrz this 3add6c7 should do the trick.

To use a uuid, just use something similar to:

  {{item['@uuid']}}

@ndejong
Copy link
Contributor Author

ndejong commented Aug 24, 2018

Most excellent indeed

@AdSchellevis
Copy link
Member

@ndejong can I close this pull request?

@ndejong
Copy link
Contributor Author

ndejong commented Aug 26, 2018

@AdSchellevis yes please do

@ndejong ndejong closed this Aug 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants