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) dev/core#2141 - OAuth2 exploratory branch #18863

Closed
wants to merge 2 commits into from

Conversation

totten
Copy link
Member

@totten totten commented Oct 27, 2020

Overview

This PR is to provide testing while the branch goes through rapid change. The aim will be to split out some parts within the week.

See also: https://lab.civicrm.org/dev/core/-/issues/2141

Before

What is the old user-interface or technical-contract (as appropriate)?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

After

What changed? What is new old user-interface or technical-contract?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

Technical Details

If the PR involves technical details/changes/considerations which would not be manifest to a casual developer skimming the above sections, please describe the details here.

Comments

Anything else you would like the reviewer to note

@civibot
Copy link

civibot bot commented Oct 27, 2020

(Standard links)

@civibot civibot bot added the master label Oct 27, 2020
@totten totten force-pushed the 5.32-oauth-imap branch 5 times, most recently from 084916f to c51bb5d Compare October 29, 2020 08:12
@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Oct 29, 2020

fail is on style warnings

@stesi561
Copy link
Contributor

stesi561 commented Nov 5, 2020

Attempting to r-run.
Enabled extension via api3
head to civicrm/admin/oauth#!/
Don't see any options to select.
Don't see any errors on console.
Don't see any errors in ConfigAndLog or watchdog - there is an info that seems to be dumping civicrm_views_tokens into Civicrm ConfigAndLog

@demeritcowboy
Copy link
Contributor

Do you just see a blank screen? It should look like this. I see it's not obvious that those words are links until you hover. I just have a habit of clicking on things even if I'm not supposed to... So just click on where it says Google Mail for example.

Untitled

@stesi561
Copy link
Contributor

stesi561 commented Nov 5, 2020

Sorry had taken a screenshot but forgot to attach!
I'm wondering if I need to include something else - I just applied -> https://patch-diff.githubusercontent.com/raw/civicrm/civicrm-core/pull/18863.patch
image

@stesi561
Copy link
Contributor

stesi561 commented Nov 5, 2020

I do see -> <!-- ngRepeat: provider in theProviders --> in the table body?

@demeritcowboy
Copy link
Contributor

You're running 5.31 and then applying this patch? I'm running master which has some of this in it, and then just applied that one other patch about the button.

@stesi561
Copy link
Contributor

stesi561 commented Nov 5, 2020

Yes tried applying it to the RC.

@demeritcowboy
Copy link
Contributor

There might be some supporting stuff in 5.32/master that it needs. To be honest not sure - I think most of this PR is in 5.32/master. There was a really short flurry of merges in the last two days.

@totten
Copy link
Member Author

totten commented Nov 5, 2020

@stesi561 Yeah, for 5.31 backport, there are probably more patches. In addition to the OAuth-specific patches, it likely needs at least #18731. (The screen with the missing data relies on that patch.)

@totten
Copy link
Member Author

totten commented Nov 5, 2020

I'm wondering if I need to include something else - I just applied -> https://patch-diff.githubusercontent.com/raw/civicrm/civicrm-core/pull/18863.patch

Oops, I didn't communicate clearly enough. :( During the previous week, I was rebasing this PR whenever bits were merged into master (5.32). Therefore, the URL above will give you the combination of all unmerged patches rather than the combination of all accumulated (merged or unmerged) patches.

For anything prior (eg 5.31-rc), one needs to backport all the PRs individually.

Since #18914 was merged into 5.32-master yesterday, we only have one more patch to merge (#18911). All of these will be in 5.32-rc (upcoming).

@eileenmcnaughton
Copy link
Contributor

@totten just a reminder that this is kinda close to the point where it should be closed

@totten
Copy link
Member Author

totten commented Nov 13, 2020

Everything in that was in this branch has either been merged in more focused PRs - or sent off to live the docs/issue-tracker.

@totten totten closed this Nov 13, 2020
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.

4 participants