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

Filtering Select Box Link fields by None #2073

Closed
jensscherbl opened this issue May 26, 2014 · 32 comments
Closed

Filtering Select Box Link fields by None #2073

jensscherbl opened this issue May 26, 2014 · 32 comments
Assignees
Milestone

Comments

@jensscherbl
Copy link
Member

Select Box Link fields that are optional can be filtered by None (publish filtering), but that doesn't seem to work.

@nilshoerrmann
Copy link
Contributor

I'm not sure if this is actually a Symphony issue or one of SBL (and the data it provides). It's the same with #2074 and #2076. Would be nice, if @brendo could have a look at this.

@jensscherbl
Copy link
Member Author

Thanks. Will have a look myself later, currently in the middle of migrating a larger project to 2.4, just wanted to log the issues before I forget about it.

@designermonkey
Copy link
Member

The issue will be that the SBL provides it's options relative to creating a select box, and the filtering won't take that into account when it calls that method.

@brendo
Copy link
Member

brendo commented Jul 28, 2014

@nilshoerrmann, how can we make the filtering get filters from the field?

I could make the SBL implement Field->displayDatasourceFilterPanel, and then the XMLElement response could be parsed so you could dynamically build the comparison operators.

I can fix this in the SBL too, but I'm not happy with it. For example, if is:None is parsed, then the SBL could look for when the value is None, apply the sql:NULL logic. If it's contains:None, that parses to regexp:None, which is a bit safer, but still not ok.

@nilshoerrmann
Copy link
Contributor

Symphony's public API is really poor whenit comes to fetching field or entry data. The biggest problem is that we have to use functions that were never created for these purposes – in this case getToggleStates which is used for the With selected … toolbar.

I'm not sure where and how this should be fixed. You know better, Brendan.
It's all a bit messy.

@jensscherbl jensscherbl mentioned this issue Sep 1, 2014
@brendo brendo modified the milestone: 2.6.0 Oct 12, 2014
@brendo
Copy link
Member

brendo commented Oct 20, 2014

@nilshoerrmann I'm thinking we need to get Field's to really implement the Field's displayDatasourceFilterPanel function. The Textbox field implements this and displays all of the filtering options to developers in the panel.

screen shot 2014-10-20 at 7 34 52 pm

If all the field's implemented this function (and by default all should at the moment with at least regexp and not-regexp options), then perhaps there is a way for you to parse this information out when you build the filter options per field?

For this immediate problem, SBL should include the magic sql: NOT NULL and sql: NULL methods. You could then pull these through the filtering interface. I'm not sure how the interface would know that these don't need any more values.

@nilshoerrmann
Copy link
Contributor

Are you available for a chat tomorrow or Wednesday?

@brendo
Copy link
Member

brendo commented Oct 20, 2014

Wednesday morning AEST works for me, how about you?

@nilshoerrmann
Copy link
Contributor

That's in the midst of the night here, if I'm not mistaken? Will have to check that first.

@brendo
Copy link
Member

brendo commented Oct 20, 2014

Ah it might be, I'm not too sure. Let me know a time that suits and I'll see what I can do :)

@nilshoerrmann
Copy link
Contributor

I just checked and your mornings are our nights (we are 8 hours behind at the moment). What about the evenings? Anything after 16:00 / 4pm your time would suit me.

@nitriques
Copy link
Member

@brendo @nilshoerrmann any conclusion was made during your chat ? I would be interested to fix that as well.

@nilshoerrmann
Copy link
Contributor

We missed each other and didn't chat yet.

The most important things in my eyes needed by the backend scripts that affect the PHP part of the system:

  • Each field should be able to provide a list of supported filters (is, contains, regexp) overriding Symphony's defaults.
  • Each field should declare, if it works string or id based, defaulting to the first.
  • Symphony should provide a method to convert between string values and entry ids (think of associative fields).

The first two points should be available in the markup or the Symphony.Context object.
The last one should be accessible through an AJAX page.

Based on these features, the UI interactions can be fixed and improved.

@nitriques
Copy link
Member

Great starting point. Do not hesitate to ping me on twitter or skype. I totally agree with your proposition.

@brendo
Copy link
Member

brendo commented Oct 31, 2014

@nilshoerrmann @nitriques Check out that commit. I've added fetchFilterableOperators to the Field class which provides this information:

'title' => 'contains',
'filter' => 'regexp: ',
'help' => __('Find values that match the given <a href="%s">MySQL regular expressions</a>.', array(
    'http://dev.mysql.com/doc/mysql/en/Regexp.html'
))

This generates a markup like:

sample datasource markup

The idea is that you could call this function from the Publish Filtering context to get the display value (title), the actual filtering (filter) and potentially any help.

Each field should declare, if it works string or id based, defaulting to the first.

Little unsure how to integrate this. The SBL works both, ID's or handles so I can't really make it pick between one or the other.

Symphony should provide a method to convert between string values and entry ids (think of associative fields).

@nitriques suggested this in #2171. It's worth noting that SBL has this with fetchIDfromValue. It's always worth saying that Field class has at least 4 methods that are all terribly named and do similar but different things: findParentRelatedEntries, findRelatedEntries, fetchAssociatedEntryIDs, fetchAssociatedEntrySearchValue. Ugh. Field class is really a great candidate for a rewrite!

@nitriques
Copy link
Member

Your proposed change looks great @brendo !

Ugh. Field class is really a great candidate for a rewrite!

Yes! I'd love to re-code it with composition in mind. But we might need to address #2171 first...

@nilshoerrmann
Copy link
Contributor

Little unsure how to integrate this. The SBL works both, ID's or handles so I can't really make it pick between one or the other.

Is that true? I wasn't aware of that. In this case: Why do handles work, but values don't? It would save us a lot of headache, if we wouldn't have to take care of ids.

@nitriques
Copy link
Member

if we wouldn't have to take care of ids.

Do you mean not having to code it or remove filtering by ids ?

IMO, I think one should be able to filter using everything that's possible: values, handles and Ids...

@nilshoerrmann
Copy link
Contributor

I was thinking of problems like #2076 where we use the id in the URL but have no way to restore the string value in the search so far.

@nitriques
Copy link
Member

ok Nils thanks.

@nilshoerrmann
Copy link
Contributor

Each field should declare, if it works string or id based, defaulting to the first.

Little unsure how to integrate this. The SBL works both, ID's or handles so I can't really make it pick between one or the other.

I thought about this over the weekend and came up with the following ideas:

  1. The type promotion idea: From SBL's perspective, using IDs should be best practice because – other than handles or values – they don't change over time. So fields could have a function that returns the prefered filtering type, e. g. value, handle or id.
  2. The type hierachy idea: As Brendan noted, fields may allow different types of filtering input. Fields could have a function that returns an array of available types, sorted by precedence. For instance, in the case of SBL this could be array('id', 'handle') which say "IDs are prefered, but handles are fine as well".

Depending on the returned value of the function, Symphony could adjust the filtering UI behaviour.

@nitriques suggested this in #2171. It's worth noting that SBL has this with fetchIDfromValue. It's always worth saying that Field class has at least 4 methods that are all terribly named and do similar but different things: findParentRelatedEntries, findRelatedEntries, fetchAssociatedEntryIDs, fetchAssociatedEntrySearchValue. Ugh. Field class is really a great candidate for a rewrite!

Thinking about the backend filtering it would be great, if we had a consistent API for retreiving and converting values or IDs on a general level: if we want to keep the filtering UI manageable, we cannot take custom field functions like SBL's fetchIDfromValue into account.


I have some spare time by the end of the week and like work on the interface aspects. If we could agree on a way how to handle the two mentioned areas that would be great (just the general concept and the naming of function, it can be implemented later).

@brendo
Copy link
Member

brendo commented Nov 4, 2014

Is that true? I wasn't aware of that. In this case: Why do handles work, but values don't? It would save us a lot of headache, if we wouldn't have to take care of ids.

Actually you can use anything. Handle's, value's or ID's will all resolve with the Select Box Link. Does that totally simplify your thoughts? This transformation happens in buildDSRetrievalSQL, so it's abstracted from you... should make Publish Filtering easy?

Don't forget to check out the filtering-fields branch for Symphony and for the SBL. Regardless of what we decide, I'd like to incorporate this in 2.6.0, if for nothing but better education of what's possible for filtering.

@nilshoerrmann
Copy link
Contributor

Yes, I'll take that branch as a base.

Regarding the SBL filtering: I remember that it didn't work with values when implementing the current version of publish filtering. Are backend filtering and DS filtering supposed to be working exactly the same?

@brendo
Copy link
Member

brendo commented Nov 4, 2014

Publish Filtering uses buildDSRetrievalSQL, so as long as it's passed in
the filter, all is well.

I've just tested this with a SBL link, while filtering will add the
suggestions and pass it through to an ID, if I change the ID to the value,
filtering continues to work.

Let me know how you go :)

On Tue, Nov 4, 2014 at 8:58 PM, Nils Hörrmann notifications@github.com
wrote:

Yes, I'll take that branch as a base.

Regarding the SBL filtering: I remember that it didn't work with values
when implementing the current version of publish filtering. Are backend
filtering and DS filtering supposed to be working exactly the same?


Reply to this email directly or view it on GitHub
#2073 (comment)
.

@nilshoerrmann
Copy link
Contributor

Okay, I'll check that out.

Let me know how you go :)

I will :)

@nitriques
Copy link
Member

This is great! I do not think that we really need a preferred filtering method, but a way to get 'supported' ones. In understand that from a DB perspective, ID are better, but from a client perspective, IDs do not even exists.

@brendo
Copy link
Member

brendo commented Nov 5, 2014

This is merged into integration. Let's push this through quickly.

@brendo brendo added this to the 2.6.0 milestone Nov 5, 2014
@nilshoerrmann nilshoerrmann self-assigned this Nov 5, 2014
@nilshoerrmann
Copy link
Contributor

I'll work on the whole filtering UI later. Looking at the latest changes for the DS filtering interface, it might be possible to unify the filter handling by also using a Duplicator in the publish area (with a different styling inside the drawer though).

@nilshoerrmann
Copy link
Contributor

The filtering branch now offers better settings for filter suggestion of associative fields that should fix the issue described in this thread.

@jensscherbl
Copy link
Member Author

Are you still working on the filtering branch? Let me know when you're done and ready for testing. Totally stunned by the amount of work you've put into this over the last few days, thanks so much.

@nilshoerrmann
Copy link
Contributor

Yes, still a few things to do. I'll create a pull request if it's ready to be tested.

@nilshoerrmann
Copy link
Contributor

This should be fixed as soon as the suggestion type is declared within SBL, see symphonycms/selectbox_link_field#80.

nitriques pushed a commit to DeuxHuitHuit/symphonycms that referenced this issue Feb 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants