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

(WIP) CRM-20160 - Remove restriction on which entities the Attachment api supports #9875

Closed
wants to merge 2 commits into from

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Feb 22, 2017

@tunbola
Copy link
Contributor

tunbola commented Feb 24, 2017

Tried this Fix on my local and was able to create an Attachment for the LeaveRequest Entity in CiviHR.

@colemanw
Copy link
Member Author

@totten can you please review this?

@colemanw colemanw requested a review from totten February 24, 2017 17:47
@totten totten changed the title CRM-20160 - Remove restriction on which entities the Attachment api supports (WIP) CRM-20160 - Remove restriction on which entities the Attachment api supports Mar 22, 2017
@totten
Copy link
Member

totten commented Mar 22, 2017

I think it's fine to continue putting discussion here, but I'm flagging this WIP because the discussion in #10010 indicates that we need major changes. If you'd rather close/reopen, that's also valid/appreciated.

@totten
Copy link
Member

totten commented Mar 22, 2017

The discussion in the other PR is a bit long, but just to capture the gist of it -- DynamicFKAuthorize::authorizeDelegate() relies on a simulated/subordinate API call. DynamicFKAuthorization uses whitelist of entities because the API call is based on soft/informal expectations. I'd expect them to hold true as a matter of best-practice, but there's no historical or design reason to think that they actually do hold across the board.

@eileenmcnaughton
Copy link
Contributor

@colemanw I'm going to close this because it appears to be long-term WIP & it seems it needs major changes. There was a suggestion half a year ago to close/re-open when ready - I think that makes sense

davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Apr 5, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Jun 26, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Jul 19, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Oct 23, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Oct 30, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Nov 9, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Dec 19, 2018
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Apr 4, 2019
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
davialexandre added a commit to compucorp/civicrm-core that referenced this pull request Jul 14, 2019
Included in CiviCRM version: N/A
Core PRs:
- civicrm#9875
- civicrm#10010
@colemanw colemanw deleted the CRM-20160 branch August 7, 2019 03:31
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.

5 participants