Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onSuccess Fires Early (and often) #76

Closed
bradsolves opened this issue Feb 28, 2021 · 5 comments
Closed

onSuccess Fires Early (and often) #76

bradsolves opened this issue Feb 28, 2021 · 5 comments

Comments

@bradsolves
Copy link

I use the following to display the progress bar, and the page refreshes constantly during the "crawl" of the progress bar and then continues to refresh indefinitely after the Celery task has completed. Perhaps my syntax is wrong?

        <script src="{% static 'celery_progress/celery_progress.js' %}"></script>
        <script>
            $(function () {
                var progressUrl = "{% url 'celery_progress:task_status' task_id %}"
                CeleryProgressBar.initProgressBar(progressUrl, {
                    onSuccess: $(function () {
                        location.reload();
                    })
                });
            });
        </script>

I am using django-celery-results as well, if that matters.

Thank you for the hard work on this application!

@OmarWKH
Copy link
Collaborator

OmarWKH commented Mar 1, 2021

I think two things are happening:

1- Reloading before success: onSuccess is responsible for updating the UI (turning the bar green). You override onSuccess, so the task is finished but the UI isn't updated. To preserve the default behaviour when overriding, call this.onSuccessDefault in your custom handler and give it all the arguments. Like this:

onSuccess: function(...args) {
  this.onSuccessDefault(...args);
  // Custom success behaviour
}

2- Continuous reloading: The task result is still stored in the backend. So when the frontend asks for task status, it finds that the task finished successfully and triggers the onSuccess handler which causes reloading. So everything is working as expected. What's your desired behaviour?

@bradsolves
Copy link
Author

Thank you very much for your prompt response. With respect to the continuous reloading, that makes perfect sense; the task result is SUCCESS so it will fire the onSuccess handler upon page load. I added some logic to check for the presence of a different UI element to determine whether the page had already reloaded, which solved that issue.

With respect tot he default handler, though, I am now receiving an error stating that this.onSuccessDefault does not exist. My script is now:

    {% if task_id %}
        <script src="{% static 'celery_progress/celery_progress.js' %}"></script>
        <script>
            $(function () {
                var progressUrl = "{% url 'celery_progress:task_status' task_id %}"
                CeleryProgressBar.initProgressBar(progressUrl, {
                    onSuccess: function(...args) {
                        this.onSuccessDefault(...args);
                        if (!$('#comparison3-tab').hasClass("disabled")) {
                            location.reload();
                        }
                    }
                });
            });
        </script>
    {% endif %}

and the error received is:

(index):1084 Uncaught (in promise) TypeError: this.onSuccessDefault is not a function
    at CeleryProgressBar.onSuccess ((index):1084)
    at CeleryProgressBar.onData (celery_progress.js:80)
    at CeleryProgressBar.connect (celery_progress.js:115)

@OmarWKH
Copy link
Collaborator

OmarWKH commented Mar 3, 2021

You are right. I was testing on the unreleased version.

The way to reference onSuccessDefault was changed in #62 but it's not included in the latest release yet.

On the current version (0.0.14), use CeleryProgressBar.onSuccessDefault.

On later versions, use this.onSuccessDefault.

@czue

@bradsolves
Copy link
Author

works great! Thanks!

@czue
Copy link
Owner

czue commented Mar 4, 2021

Sorry about that!

I just pushed a new release (0.1.0 - because I don't really know why I ever started with 0.0... 🤷‍♂️ ). Feel free to install that one and let me know if there are any issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants