Apache Allura™

Hi.. That is nice idea.. I will take a look. We need to call edit endpoint in background. Also if the post is new one, nothing needs to be happend

I checked about those existing endpoints issue is that looks like unable to pass only mardown. For example wiki update requires title, labels, etc.. if we use existing endpoint we need to pass those with ajax req too.. Can we create some endpoints just to update markdown for checklist click.. your idea?

Good point about existing fields. We can create new endpoints. They could be part of our official REST API. The PATCH http method (instead of PUT/POST) is a good one for just updating a single field. I don't think a single API endpoint for all types of artifacts would work, because each one (ticket, wiki page, comment, blog post, etc) has a different way of being identified/named and stores its markdown in different fields. So we would probably have to update many existing APIs that accept POST to accept PATCH too. And probably create a few APIs if they don't have one yet.

It sounds like quite a bit of work, but if you wanted to do it, I'd suggest starting with just one like a ticket. Then we could work through all the details of that before applying the pattern to all the other places.

Hi. Dave

Thanks. I will start for one module. sounds wiki is good start

Hi.. Dave

Can you check this approach https://forge-allura.apache.org/u/shalithasuranga/allura/ci/868a0510030d7d42d7a5913c5c152224fa884dae/

This works well for wikis but permission check is not enabled with this commit. I tried simply has_access(self.page, 'edit') but it always returned False. I need to check the code of has_access

Your ideas ?

has_access(self.page, 'edit') I think should work. The implementation of has_access has a lot going on, so might be hard to understand all of it. One thing you can try is passing c.user as the user parameter, it can be good to be explicit but has_access should be defaulting to that anyway.

I tried your commit and it works, so that is a good start for sure :) I see in the wiki page history that the edits are done by "Anonymous" so that's probably related to why the permission check fails.

All form POSTs must include a CSRF token called session id, for security. Without it, they are treated as anonymous, so I think that is what is happening. If you search our codebase for $.cookie('_session_id') you can find examples of JS form submits using it.

The Checklists JS code could be moved to a more specific location. I believe it can be in page_view.html instead of master.html since it only needs to be used when a page is shown. And if there are comments on a page, then each comment has its own class="markdown_content" so the .markdown_content selector may have issues if it is matching multiple things. It seems ok now but something to keep in mind as this expands into other things, including handling comments themselves.

Hi.. Dave

Thank you for the guidance above content was really helped. I have made changes as per below

• Added edit permission check if user has no permission displayed a notification saying "Changes will not persist"
• Added activity entry as "modified wiki page"
• Tested and no more modifications from anonymous user
• Moved checklist js code from master to page view
• Used only for wiki page content
• Added spam checker line

Here is my idea. Once we make this properly for wiki, We will make for other non-repeating things like ticket, blog, forum post. but threads are repeating We need to find a proper way to add checklist js sninppet. One easy way is we can add inside the loop of threads but there will be more js.

Please check latest test commit https://forge-allura.apache.org/u/shalithasuranga/allura/ci/test/

Thanks

Cool. Good job adding the things like activity entry, I had forgotten about that but that is good. I think it'd be a little better to not let people even check the boxes if they don't have permission. For example add a has_access check in "block wiki_extra_js" and don't do the JS if they don't have permission. Still keep the check in update_markdown too though.

I agree doing it for something like ticket next would be good. That will help see what similarities there are between each thing and then can set up jinja macros or python functions to share the similar code.

When you get to discussion threads later, doing it inside the loops would be too much repeated JS for sure. I think we should try to put the JS code in once for the whole discussion thread and have it be smart enough to work for any/all of the comments. But we can look into that more when we get there.

Thank you. I will make some commits for the things as you explained above. And once we agree with general approach we will add for other non-repeacting things. For threads we will discuss an approach when we complete works for all of non-repeating things

Hi Dave

I did the permission check modification. Btw Now I came with a general solution for all modules including threads using same single js code without putting inside loops.

Here is the way.

Each markdown_content we wrap with class="active-md" data-markdownlink="" data-markdownlink has ajax endpoint for checklist js snippet. So for main modules's value of data-markdownlink is "" so it will call .../update_markdown ../get_markdown etc. But for the threads it's different we need to add value.url() to get corrent endpoint so for those it will call relevant urls such as /p/allura/discussion/general/thread/86d45b7f/4194/update_markdown

Please check latest commit diff and review this approach https://forge-allura.apache.org/u/shalithasuranga/allura/ci/test/

If it looks good we can easiliy add for all modules. we just need to add get_markdown and update_markdown with relevant permissions and also just need to add js code as extra js snippet

Thanks

Hi..

Any update regarding this. Can I continue with above approach ?

Thanks

Done in https://forge-allura.apache.org/p/allura/git/merge-requests/295/