[aerogear-dev] Push Admin UI - Updating

Tue Jul 16 08:30:10 EDT 2013

On Tue, Jul 16, 2013 at 2:23 PM, Kris Borchers <kris at redhat.com> wrote:

>
> On Jul 16, 2013, at 7:20 AM, Matthias Wessendorf <matzew at apache.org>
> wrote:
>
>
>
>
> On Tue, Jul 16, 2013 at 2:14 PM, Kris Borchers <kris at redhat.com> wrote:
>
>>
>> On Jul 16, 2013, at 7:10 AM, Matthias Wessendorf <matzew at apache.org>
>> wrote:
>>
>>
>>
>>
>> On Tue, Jul 16, 2013 at 2:05 PM, Kris Borchers <kris at redhat.com> wrote:
>>
>>>
>>> On Jul 16, 2013, at 3:05 AM, Matthias Wessendorf <matzew at apache.org>
>>> wrote:
>>>
>>>
>>>
>>>
>>> On Tue, Jul 16, 2013 at 9:57 AM, Sebastien Blanc <scm.blanc at gmail.com>wrote:
>>>
>>>>
>>>>
>>>>
>>>> On Tue, Jul 16, 2013 at 8:36 AM, Matthias Wessendorf <matzew at apache.org
>>>> > wrote:
>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Jul 15, 2013 at 8:34 PM, Lucas Holmquist <lholmqui at redhat.com>wrote:
>>>>>
>>>>>> here's a scenario.
>>>>>>
>>>>>> A user creates an "Application" then adds a new "iOS Variant" to it.
>>>>>>
>>>>>> they decide they need to rename this Variant since they didn't name
>>>>>> it very well.
>>>>>>
>>>>>> There is currently no way of renaming/ updating the description of
>>>>>> this variant without re-sending the Certificate/pass phrase.  this would be
>>>>>> the same for Android and Simple Push,  just substitute google key and
>>>>>> channels
>>>>>>
>>>>>
>>>>> yes - but should be easy to change :-)
>>>>>
>>>>> See here:
>>>>>
>>>>> https://github.com/aerogear/aerogear-unified-push-server/blob/master/src/main/java/org/jboss/aerogear/connectivity/rest/registry/applications/AndroidVariantEndpoint.java#L134-L138
>>>>>
>>>>> in the "update", we just need to onbly update/replace those things
>>>>> that are NOT null. Of course, perhaps we need to rethink the "update
>>>>> validation" too:
>>>>>
>>>>> https://github.com/aerogear/aerogear-unified-push-server/blob/master/src/main/java/org/jboss/aerogear/connectivity/rest/registry/applications/AndroidVariantEndpoint.java#L130-L132
>>>>>
>>>>> Yes that could work even if I must admit I'm not a big fan of updating
>>>> based on a null check ... Imagine someone wants to delete the description
>>>> by setting it to null , that won't be possible ;)
>>>>
>>>
>>> well, yeah - that (the desc) is optional - I was more interested in the
>>> required fields:
>>> e.g. for iOS Variant:
>>> * cert/passphrase (or googlekey/networkURL on other variants)
>>> PushApp:
>>> * name
>>>
>>>
>>> I'm not sure how I feel about this partial update. I'm not a big fan of
>>> saying "It's a required field but leave it null if you don't want it to
>>> change." Feels wrong.
>>>
>>
>> not sure I am following, but the required fields should never be null
>>
>>
>> Now I'm not following.
>>
>
> :-)
>
>
>> Let's go through a scenario and see how each would work with your method.
>> Imagine there are currently values in every field. What fields need values
>> and what can be null when:
>>
>> I update a single required field?
>> I update a single optional field?
>>
>
> Well,when I update the FORM and hit ENTER (let's assume this is a PUT,
> just for now..), aren't the entire form values sent?
>
> So, any field that changed will be udpated, I think
>
> Now if you NULL out a required field, I'd imagine that is not allowed.
>
>
> So I guess I'm confused as to why there was any discussion of NULL fields
> being ignored then. If the form is filled in for the user, if they empty a
> field it should either fail (required field) or empty the value (optional
> field).
>

+1

> NULL should never just be ignored, right?
>

I think, the "confusion" comes in..... what happens if (iOS Variant), JUST
the name is changed (note: the certificate FILE is required).

I guess that would be just fine... if (on the JUST iOSVariant.name is
changed), we do not bother with the certificate.

We update the certificate ONLY if it is actually part of the "UPDATE
request".

-M

>
>
>
>
>
>>
>>
>>
>>
>>>
>>> If we are to do partial updates like this though, I would say it should
>>> probably use http PATCH instead of PUT, right?
>>>
>>>
>>>
>>>
>>>
>>>
>>>>  (Well not sure it is a valid use case but you see my point) and as
>>>> you said we must then rethink user validation.
>>>> Other option is to split the update into more smaller specific updates
>>>> methods ... but not sure about this also.
>>>>
>>>
>>>
>>>
>>>
>>>
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Also,  once we have a variant created,  we have a variant ID,  we
>>>>>> should be able to use that ID( plus type? ) to do updates instead of having
>>>>>> to pass the applicaitonID and variantID( same would go for a delete i guess
>>>>>> ).
>>>>>>
>>>>>>
>>>>>> Not really sure the best way to handle this.
>>>>>>
>>>>>
>>>>> the "variantID" is not a PK, but it's unique - so a new finder is what
>>>>> you need.
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Should a user be able to update a certificate/key/channel once it has
>>>>>> been created.
>>>>>
>>>>>
>>>>> yes, they should be able to replace an outdated (or revoked)
>>>>> certificate, API etc.
>>>>>
>>>>>
>>>>>
>>>>>>  How would this affect any installations it might have?
>>>>>>
>>>>>
>>>>> Not at all.  so if (speaking Androind) ... a new "project" is used -
>>>>> there is a new SenderID - that's more impact, since the actual mobile app
>>>>> needs to be updated, but again that's not really a concern for the push
>>>>> server
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> thoughts?
>>>>>>
>>>>>> -Luke
>>>>>> _______________________________________________
>>>>>> aerogear-dev mailing list
>>>>>> aerogear-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matthias Wessendorf
>>>>>
>>>>> blog: http://matthiaswessendorf.wordpress.com/
>>>>> sessions: http://www.slideshare.net/mwessendorf
>>>>> twitter: http://twitter.com/mwessendorf
>>>>>
>>>>> _______________________________________________
>>>>> aerogear-dev mailing list
>>>>> aerogear-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> aerogear-dev mailing list
>>>> aerogear-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>>
>>>
>>>
>>>
>>> --
>>> Matthias Wessendorf
>>>
>>> blog: http://matthiaswessendorf.wordpress.com/
>>> sessions: http://www.slideshare.net/mwessendorf
>>> twitter: http://twitter.com/mwessendorf
>>> _______________________________________________
>>> aerogear-dev mailing list
>>> aerogear-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> aerogear-dev mailing list
>>> aerogear-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>
>>
>>
>>
>> --
>> Matthias Wessendorf
>>
>> blog: http://matthiaswessendorf.wordpress.com/
>> sessions: http://www.slideshare.net/mwessendorf
>> twitter: http://twitter.com/mwessendorf
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>
>>
>>
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>
>
>
>
> --
> Matthias Wessendorf
>
> blog: http://matthiaswessendorf.wordpress.com/
> sessions: http://www.slideshare.net/mwessendorf
> twitter: http://twitter.com/mwessendorf
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
>
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>

-- 
Matthias Wessendorf

blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20130716/133120fd/attachment-0001.html 