lol768 / XenForo-MCASSOC

Minecraft association addon for XenForo
Other
20 stars 10 forks source link

Fix "just-created" messages not showing association info #7

Closed lol768 closed 9 years ago

lol768 commented 9 years ago

@PunKeel on #3

Hi,

When you reply to a PM, the "dynamically added block" (with your sent message) does not include the Minecraft part. Hadn't time to try myself to fix it, but I report here meanwhile

@lol768 on #3

I'm aware of this one since it's something I have looked into without much luck in the past. This should also probably apply to messages made in threads too. I'll have another look at some point in the future but since XenForo doesn't do much in the way of developer documentation I find writing addons for it difficult.

punkeel commented 9 years ago

After some digging, (using PM, haven't tested with threads) : the ajax request returns the raw html part, rendered from conversation_view_new_messages, itself looping over conversation_message, containing an <xen:include template="message"> (and message itself calling the message_user_info template, and so your modification)..

In a word, everything is like when it's not AJAX. :)

Epicness level ... 9000 ?

EDIT: As the route "used" for posting a reply is not index, I think it might have an impact.

I agree with myself, actionIndex() is not fired during reply posting (in AssociationMc_ControllerPublic_PrivateMessage)

lol768 commented 9 years ago

@PunKeel That's really helpful, thanks!

I take it you meant actionView() since I can't see an actionIndex() in that controller (or at least a function that I am overriding). In any case, I think you've hit the nail on the head with the AJAX-y stuff not causing that function to be called.

I'm guessing I need to look at overriding actionInsertReply() from XenForo_ControllerPublic_Conversation. Will do some experimentation.

Thanks again.

punkeel commented 9 years ago

Really ?

:heart:

lol768 commented 9 years ago

Yup! Thanks a bunch for your help. Release will be on XenForo's site soon, too (edit: https://xenforo.com/community/threads/associationmc.74920/#post-904639).

https://github.com/lol768/XenForo-MCASSOC/releases/tag/1.0.1

punkeel commented 9 years ago

Patch applied & bug fixed :D

Thanks again !

lol768 commented 9 years ago

Awesome, let me know if you run into any other issues or have any requests.

punkeel commented 9 years ago

Simple one, not linked to your plugin but more to Xenforo: how am I supposed to update a plugin (I've manually applied the patch). I've seen a plugin that could handle it, but it requires my Xenforo credentials, and I don't really like it ... [I grant myself the right to ask here because you could include an auto updater, but it looks like it's uncommon in the great Xenforo's world ...]

lol768 commented 9 years ago

The way I do it locally is like this:

What would be nice is automating the final part - I could look into a script to do the pulling and upgrading in one step, perhaps.

Yeah, auto-updaters are uncommon and with the latest drama over addons phoning home I'd probably prefer not to risk integrating one.

Do you have SSH access to the server you host the forums on? None of the above is going to work if you don't, unfortunately - and I know a lot of people in the XenForo world do just rely solely on shared hosting and use FTP etc to upload the files when things change.

punkeel commented 9 years ago

I own my dedicated server, so I do have git, ssh ... and I'll stick with this method :-)

Thanks again !

(btw, as a side note ... full command to be done in ./library/ is git clone https://github.com/lol768/XenForo-MCASSOC.git AssociationMc, I think it wouldn't work with the repo name)

punkeel commented 9 years ago

Oh, talking about drama, I use one of their free plugins (downloaded yesterday, never ever heard about the drama ... And I don't really care, the source code seemed clean at first sight) that live refreshing. And it doesn't work with your addon :( The called url being {/forum}/threads/thread-update.json, I think the issue is the same as in this issue ... but I'm unsure if it's a good idea to add every route from every addon ... Maybe is there a "global" route for this purpose ?

If no content is added, the reply is the same as insertReply:

{"templateHtml":"","css":"","js":"","_visitor_conversationsUnread":"0","_visitor_alertsUnread":"0"}
lol768 commented 9 years ago

(btw, as a side note ... full command to be done in ./library/ is git clone https://github.com/lol768/XenForo-MCASSOC.git AssociationMc, I think it wouldn't work with the repo name)

Yeah, that's a good point - I'm bad with consistency sometimes..

With regard to that addon:

Anything that adds new methods of grabbing the current posts info instead of interacting with (i.e. extending) the normal ways of grabbing new posts and/or threads is probably going to conflict with how I'm doing things. I'd need to look into the addon itself, since unfortunately they could be doing it in a number of different ways. The only way I can think of doing it that would be 'global' would be to use a template hook instead of extending the controller methods. Unfortunately, this would be a big performance hit (i.e. one query per post) because I can't just use one query for the entire page any more.

punkeel commented 9 years ago

I thought they used some kind of common api, like adding the format at the end of the url.

But no, they call their own route ({/forum}/threads/thread-update.json is the true url, and so does not include the thread id ...)

Thanks,