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

[184790111]: Redirect Crunch Automation on root project folder #621

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sluga
Copy link

@sluga sluga commented Apr 17, 2023

No description provided.

@sluga sluga requested a review from gergness April 17, 2023 16:26
@sluga sluga force-pushed the add-Crunch-Automation-on-the-account-#184790111 branch from e9b48c0 to 8ea945a Compare April 17, 2023 16:41
@@ -206,18 +206,21 @@ setMethod("sendCrunchAutomationScript", "ProjectFolder", function(x,
# which gives us the URL to hit;
# but the account ('top-level folder', what you get from: `projects()`)
# is also of class ProjectFolder, but doesn't include this info;
# running CA scripts on the account is not supported currently
# previously, we function raised an error in this case;
# now, following what frontend did, we execute the script on the account
if (!is.crunchURL(x@views$execute)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should do some heuristics to only redirect when at the root and keep the error if we're not and there's no execute view (I don't know if it currently works this way, but I can imagine the API not letting you run scripts on certain folders because of permissions by not including the execute view, and we wouldn't want to silently redirect to the account in this case).

Is there an obvious way to tell? If not, we can ask Jose what the frontend does here: https://github.com/Crunch-io/whaam/pull/8006/files#diff-ed5a738f637f797570efe80b7388b58ef741de4cb66bf7615caf498b3b35978bR27

@@ -0,0 +1,6 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is how I was imagining getting around the error in GET before the expect_POST. It seems like it works for me, do you still get errors? (If so, did you run make compress-fixtures?)

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

Successfully merging this pull request may close these issues.

2 participants