rhomobile / rhodes

The Rhodes framework is a platform for building locally executing, device-optimized mobile applications for all major smartphone devices.
http://tau-platform.com/
MIT License
1.05k stars 237 forks source link

RhoModel#delete_all ignoring string query values #975

Closed leonardofalk closed 5 years ago

leonardofalk commented 5 years ago

This works as expected, deletes only Models with some_field = null.

Model.delete_all(conditions: { some_field: nil })

This dot not works as expected, it deletes every model record.

Model.delete_all(conditions: 'some_field is null').

Current workaround is Model.find(:all, conditions: 'some_field is null').each(&:destroy).

The API don't mention the conditions as a query string, but .findmethod allows it and works, so I don't know if this is a regression problem or it never meant to work like this.

vmusulainen commented 5 years ago

Leonard,

Syntax like Model.delete_all(conditions: 'some_field is null'). is wrong.

I didn't find any mention this syntax at documentation

Should use syntax like Model.delete_all(:conditions => {'<your field>' => '<value>'})

leonardofalk commented 5 years ago

@vmusulainen yes, I've mentioned that it's not in docs. However it works when using .find(conditions: ' ... '). As the operation is destructive, this should be documented or the user should be warned when the parameters are not as expected.

vmusulainen commented 5 years ago

@leonardofalk, .find method does not officially support syntax with query string. So .find with query string is undocumented feature and we don't recommend to use it. Thus, what is the point of warning about this, if documentation says clearly how to use these methods in the right way?

leonardofalk commented 5 years ago

@vmusulainen Ok, that seems fair. This project was born in rhodes v2, and I'm working with it since v4 and I thought it was a undocumented regression error, because it should've worked back then. That being said I'll close this issue. Thank you for your time.