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

Afform - Fix loading entities from url args #20145

Merged
merged 1 commit into from
May 6, 2021

Conversation

colemanw
Copy link
Member

Overview

Fixes https://lab.civicrm.org/dev/core/-/issues/2567

Loading existing entities via url args was not working.

Before

Url arguments ignored.

After

Now entities can be loaded with a url like civicrm/my/afform#!?Individual1=123

Comments

This broke when we stopped using ngRoute.

This broke when we stopped using ngRoute.
Now entities can be loaded with a url like civicrm/my/afform#!?Individual1=123
@civibot
Copy link

civibot bot commented Apr 26, 2021

(Standard links)

@civibot civibot bot added the master label Apr 26, 2021
@eileenmcnaughton
Copy link
Contributor

#!?

image

@seamuslee001
Copy link
Contributor

@colemanw I tried this out locally but ran into a problem where by when it was trying to do the prefill with args of {Individual1: "198"} got error Cannot process APIv4 request for Individual1 (Contact.get): Action is not approved

@colemanw
Copy link
Member Author

@seamuslee001 what the #!? does that error mean?

@colemanw
Copy link
Member Author

You know what @eileenmcnaughton - your comment got me thinking and I decided the best thing is to remove that ! character globally, not just on pages that use crmApp.
#20152

@seamuslee001
Copy link
Contributor

@colemanw that error seems to come from here

throw new UnauthorizedException("Cannot process APIv4 request for $entityName ($entity.$action): Action is not approved");
maybe the recent RBAC/FBAC work that Tim did broke prefill somehow ?

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 @colemanw - just trying to figure out the status here - is it blocked on #20152 ? I guess more directly it's blocked on the issue @seamuslee001 hit

@colemanw
Copy link
Member Author

I don't think #20152 is a blocker per-se but I think it ought to be merged. @totten can you please take a look?
I'll try to reproduce the issue Seamus hit.

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 do you still think there is an error with this PR? I think it would be good to hit the rc before it forks as this is really a regression

@eileenmcnaughton
Copy link
Contributor

I tested this & was able to get this to work

https://dmaster.localhost:32353/civicrm/monolog#!?Monolog1=1

There are still some issues in getting that form to fully work but this PR fixes this part & it is actually a regression so let's get it merged before forking

I didn't experience the bug @seamuslee001 did - but we can merge a fix to that to the rc if that is a regression from this or a pre-existing regression

@eileenmcnaughton eileenmcnaughton merged commit 5032866 into civicrm:master May 6, 2021
@eileenmcnaughton eileenmcnaughton deleted the afformRouteParams branch May 6, 2021 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants