Fork me on GitHub
#untangled
<
2016-09-26
>
ethangracer00:09:28

@adambros @mitchelkuijpers we haven't implemented access to Om shared because in several discussions with people on the channel we have always arrived at design alternatives that were easier to reason about

mitchelkuijpers07:09:54

@ethangracer Yeah I also came tot that conclusion by just using a few globals and global atoms

mitchelkuijpers09:09:55

How does the loading marker work when you load a collection? I cannot seem to get a hold of it

ethangracer13:09:04

@mitchelkuijpers you can add [:ui/loading-data ‘_] to the query of any component whose query composes to root. It’s a boolean value set to true when a data fetch is in progress

mitchelkuijpers13:09:30

Yeah, I know but I want to know if a certain query is running

mitchelkuijpers13:09:54

But I noticed something strange, it won’t add the loading indicator when i add params

mitchelkuijpers13:09:01

and it adds nil nil in the app state

ethangracer13:09:17

we knew about the nil nil, not sure we caught the loading indicator too

mitchelkuijpers13:09:41

Yeah I saw the nil nil in the issues

ethangracer13:09:05

the loading indicator should show up in the location in app-state where the data will be placed

ethangracer13:09:14

if it isn’t, then you should file a bug

mitchelkuijpers13:09:20

Yeah, it only does when I remove the params

mitchelkuijpers13:09:33

The two are probably related

ethangracer13:09:39

yeah definitely throw a minimal repro up as a github issue and I’ll take a look

ethangracer13:09:56

yeah, thank you

ethangracer13:09:09

@mitchelkuijpers interesting, even when :marker is set to false?

ethangracer13:09:18

if :marker is false there should never be a ui fetch state

ethangracer13:09:44

yeah, if :marker is not specified, it defaults to true

ethangracer13:09:00

cool, I’ll play around. thanks

ethangracer13:09:55

@mitchelkuijpers I pulled down your repo and am trying to run it, but there don’t appear to be any html files in the resources folder… ?

ethangracer15:09:03

@mitchelkuijpers you’re right, they are related

ethangracer15:09:09

There’s a bug in untangled.client.impl.data-fetch/data-query-key where it returns nil on a parameterized query. So a load marker is put under key nil and cleared out once the load is completed.

mitchelkuijpers15:09:49

Aha that explains the nil key

ethangracer15:09:55

which is also why the load marker isn’t in the right place

ethangracer15:09:01

two birds one stone 🙂

mitchelkuijpers15:09:30

Would you like me to submit a pull request?

mitchelkuijpers15:09:37

I would be happy to fix it

ethangracer15:09:45

Thanks for offering, should be pretty quick I have this one

mitchelkuijpers15:09:08

I think you might be fixing 3 issues

mitchelkuijpers15:09:22

this one is also related?

ethangracer15:09:34

I think I got that one

mitchelkuijpers15:09:58

We’ll this will be a good fix then, i liked them to the issue for you

ethangracer15:09:17

the logic in those files could use some serious code tuning

ethangracer15:09:35

@tony.kay another project during documentation week, perhaps, would be to explore simplification of the data fetch logic

mitchelkuijpers15:09:49

No problemo, happy to help. If you guys ever have some small things that needs fixing but no time be sure to give me a heads up. Would love to contribute some work back

ethangracer15:09:58

awesome! that’s much appreciated

ethangracer15:09:45

just writing some tests and I’ll send it up to github. we’ll have to wait for tony for a snapshot update

tony.kay15:09:45

@ethangracer hit me with a PR on that pls

ethangracer16:09:19

untangled spec is giving me a hard time for some reason

ethangracer16:09:11

I can’t get the tests to run so I’m going to submit the patch as a PR

ethangracer16:09:20

I’ll fix the testing when I refactor

tony.kay16:09:36

commented out at the moment and manually tested?

mitchelkuijpers16:09:45

Is there a way to just unmount an untangled app and mount it again? Our om.next solution was add-root! and remove-root! I am integrating with a bigger application and I am having a hard time changing the root dom-node

tony.kay16:09:55

you can use Om's remove-root!...it is an Om app

tony.kay16:09:17

the new reset stuff might be needed for remounting...have not tried it

ethangracer16:09:19

Something appears to be broken with our Travis CI build

ethangracer16:09:26

not running the tests

tony.kay16:09:39

travis Firefox is kinda flaky...sometimes fails for bad reasons like timeouts

tony.kay17:09:09

0.5.7-SNAPSHOT is on clojars of client. Should fix a number of load marker bugs

tony.kay17:09:46

@mitchelkuijpers I would recommend that you avoid loads triggered from component mount, since React can do mount/unmount at unexpected times

tony.kay18:09:12

You're better off doing the loads from the operation that led to your UI change that leads to the mount of that component

tony.kay18:09:14

that way arbitrary UI changes that lead React to think one thing is going away and another is coming in (where in fact they are the same thing) won't cause subtle bugs

tony.kay18:09:30

If anyone knows how to get Travis CI to do a better job of running tests through Firefox, let me know. Our tests pass, but when Travis runs Firefox it often times out and counts all tests as failing

mitchelkuijpers18:09:27

Yeah I don't do that. This was only a small playground

mitchelkuijpers18:09:02

Thanks for the snapshot!

jasonjckn18:09:34

let's say in the UI the user can 'update the status of object ID <id> to <NEW_STATUS>' in the backend this is idempotent, it tries to succeed regardless in a number of scenarios e.g. (1) status is updated (2) status was already set to new status (3) it did some workaround in an error case, and I want feedback into the UI about which path it took to succeed

jasonjckn18:09:41

is this a load field action + api-read ?

jasonjckn18:09:47

since there's data flowing from server to client

jasonjckn18:09:53

(the feedback)

mitchelkuijpers18:09:49

Btw is it possible to get the result of a remote mutation (the response?)

mitchelkuijpers19:09:22

@ethangracer I can confirm that the fix works 🙂 thank you so much

ethangracer19:09:50

your welcome. glad you’re enjoying untangled so far

mitchelkuijpers19:09:05

Yeah I am currently deleting all of my om.next reads

adambrosio20:09:49

@mitchelkuijpers i dont think we let you access the response of a mutation, but there is technically a untangled.client.mutations/post-mutate that gets called after a mutation’s :action https://github.com/untangled-web/untangled-client/blob/b5b1106ae89470651c040af3469652f6033f1b8c/src/untangled/client/impl/om_plumbing.cljs#L38

adambrosio20:09:24

what are you wanting the result of a mutation for?

mitchelkuijpers20:09:59

I am porting some code where we add something to a list and then clientside we have to call a method to update a counter, so that mutation returns the data we need to calculate that counter (we need to do this client-side, because of reason with atlassian-connect integration)

mitchelkuijpers20:09:40

But maybe we should just add a new read to get that information

adambrosio20:09:14

yeah you could either figure out how to make the response available, or you just need to add a level of indirection and have a server side counter

adambrosio20:09:29

then you could just [(inc-counter) :counter]

mitchelkuijpers20:09:54

Haha i know the escaping of mutations 😉

adambrosio20:09:03

are you talking directly to atlassian? haha yeah i couldnt figure it out

adambrosio20:09:01

hmm well then im not sure what your best option is, but i can tell you that you’d be poking around here: https://github.com/untangled-web/untangled-client/blob/26c1844958433dbcee0838a5521fb1885b485a3e/src/untangled/client/core.cljs#L104 and making the :response-channel optionally pluggable

adambrosio20:09:37

either cb, a multimethod or something

mitchelkuijpers20:09:09

I could fix this with a read so I might try that

adambrosio20:09:43

try that, but it could be useful to have a story for mutating non untangled-server api’s and wanting the response

mitchelkuijpers20:09:45

I think ill just do an optimistic update 🙂

adambrosio20:09:51

hehe yeah that too

mitchelkuijpers20:09:03

then I can just revert when necessary

mitchelkuijpers20:09:01

But still it might be usefull indeed

mitchelkuijpers20:09:33

I expected that the post-mutation would have it

adambrosio20:09:35

we’ll be having a documentation/refactoring iteration in a week or two, so i could take care of that easily/quickly if you need it

adambrosio20:09:52

hell i could do it now if you really need it

mitchelkuijpers20:09:14

I can wait I’ll change it to an optimistic update (I find that a nice solution)

mitchelkuijpers20:09:24

But it would be a nice improvement

mitchelkuijpers20:09:32

Do you want me to create an issue?

adambrosio20:09:50

i think tony may be of the opinion you should normally be doing an optimistic update followed by reads

adambrosio20:09:57

but feel free to make one and we can discuss this there

ethangracer21:09:50

@adambros @mitchelkuijpers the intended design pattern is to do the remote mutation followed immediately by a remote fetch

ethangracer21:09:16

e.g [(remote/mutation) (untangled/load-data query)]

ethangracer21:09:34

mutations only return tempids

adambrosio21:09:16

right, but how does it work if you talk directly to a non-untangled server?

adambrosio21:09:26

esp one that you dont control

ethangracer21:09:49

wouldn’t that just be a standard http post?

ethangracer21:09:01

I suppose you could build a custom untangled networking object

adambrosio21:09:02

if i think about it @mitchelkuijpers must have an untangled-server, so i dont know what he meant by he talks directly to atlassian

adambrosio21:09:44

i suppose, but what if you also have an untangled server?

ethangracer21:09:25

yeah you’d have to have something like multiple om remotes

adambrosio21:09:42

yeah, which we dont support yet

adambrosio21:09:02

anyway i think the recommended route is to have an untangled server communicate with these other services

grzm21:09:09

For another data-point, I'm using untangled-client with my own (pedestal-based) server. It's working fine. I just make sure I return the tempid mapping.

tony.kay21:09:24

@adambros @mitchelkuijpers @ethangracer Ethan is correct here. The Om model is that mutations can remap tempids. There is no return value. The reason for this is that there is no query, so there is no way to merge that result into your database. Thus, the follow-on remote read

tony.kay21:09:41

This is a common misconception about the overall model

tony.kay21:09:41

The model is that the mutation is abstract. The UI and Server have no coupling. The server has no idea what you're showing, and the client has little idea (other than advertised keywords on the mutation...which isn't documented well, but is part of Om) what the server is really doing.

tony.kay21:09:04

So, in order to fix this gap, the client specifically asks for reads on the data that it knows it is displaying that could have changed on the server

tony.kay21:09:19

The mutation would have to "cover the bases" to ahve a return value (and it would have to have a normalization query)

tony.kay21:09:51

Say you have a mutation add-friend. The server could return all manner of information: the new friend count, a success code, the list of current friends

tony.kay21:09:09

Now you're tempted to make a mutaiton for every possible return value? Or just send it all back?

tony.kay21:09:27

None of that is nearly as simple as the UI directly saying: do this, and then I need to know this other thing

tony.kay21:09:26

If you read the Om docs David mentions listing keys on a mutation. These are where you put advisory documentation on your mutation as to what data items the mutation affects, so that a UI programmer can read that (as documentation) and decide what to explicitly query

tony.kay21:09:27

I'd recommend documenting the affected keys in whatever way makes sense for your developers, because this is a point of difficulty (and maintenance), though in practical systems it is pretty obvious what things you might want to read because the mutation is an abstraction with general meaning.

tony.kay21:09:45

E.g. in the add-friend case I can guess that my friend count just changed.

tony.kay21:09:17

It is a subtle shift, and I understand the desire to want a "return value"...but they just don't make sense in the overall model

tony.kay21:09:00

(and I'd argue that David is right here...this is a simpler/better approach to the problem of UI composition)

tony.kay21:09:36

I really need to write this all down in a Blog entry or something 😄

therabidbanana22:09:42

I think that explanation helped me a lot - a blog post about the model would be great. 🙂

tony.kay22:09:44

oh boy...yeah, sounds like I really need to do it if you've written a complete commercial app and you're just getting it 😜

therabidbanana23:09:50

Heh, yeah - I generally understand that mutations don't return anything, but I've never really tied together the reasoning behind it is specifically "you don't know what to return because that's what reads are for"