Skip to content

No Authorization in ReactRedux #11801

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.

Sign up for GitHub

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

Jump to bottom
Merged
mkArtakMSFT merged 2 commits into from
Jul 3, 2019
Merged

No Authorization in ReactRedux #11801

mkArtakMSFT merged 2 commits into from
Jul 3, 2019

Conversation

@ryanbrandenburg
Copy link
Contributor

@ryanbrandenburg ryanbrandenburg commented Jul 2, 2019

The inclusion of this attribute causes all calls to this endpoint to fail because there is no authorization system registered.

@javiercn
Copy link
Member

javiercn commented Jul 2, 2019

You can also remove the using

@javiercn
Copy link
Member

javiercn commented Jul 2, 2019

Unflaky this test also https://github.com/aspnet/AspNetCore/blob/release/3.0-preview7/src/ProjectTemplates/test/SpaTemplateTest/ReactReduxTemplateTest.cs#L22

@javiercn
Copy link
Member

javiercn commented Jul 2, 2019

We didn't catch this regression because the test was skipped and marked as flaky. There is no functional difference between this test and the react one, which runs fine, so this shouldn't be flaky.

javiercn
javiercn requested changes Jul 2, 2019
View reviewed changes
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Overall looks good, but I want the relevant tests to be enabled and run for these changes and for the additional namespace to be cleaned up.

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 2, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview7 milestone Jul 2, 2019
@mkArtakMSFT
Copy link
Contributor

mkArtakMSFT commented Jul 2, 2019

This is approved for Preview7 release. Please merge as soon as tests are enabled and pass.

@ryanbrandenburg ryanbrandenburg requested a review from javiercn July 2, 2019 17:35
mkArtakMSFT
mkArtakMSFT approved these changes Jul 2, 2019
View reviewed changes
Copy link
Contributor

@mkArtakMSFT mkArtakMSFT left a comment

Choose a reason for hiding this comment

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

Thanks Ryan!

@ryanbrandenburg
Copy link
Contributor Author

ryanbrandenburg commented Jul 2, 2019

Not sure why this is failing, I tested it on a template locally and it worked fine, investigating.

@ryanbrandenburg
Copy link
Contributor Author

ryanbrandenburg commented Jul 2, 2019

Ah, so the failure seems to have been a transient thing in an Angular test, retrying.

@ryanbrandenburg
Copy link
Contributor Author

ryanbrandenburg commented Jul 2, 2019

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=247903

@aspnet-hello
Copy link

aspnet-hello commented Jul 2, 2019

This comment was made automatically. If there is a problem contact [email protected].

I've triaged the above build. I've created/commented on the following issue(s)
https://github.com/aspnet/AspNetCore-Internal/issues/2609
https://github.com/aspnet/AspNetCore-Internal/issues/2489
https://github.com/aspnet/AspNetCore-Internal/issues/1979

@mkArtakMSFT mkArtakMSFT merged commit c966fff into release/3.0-preview7 Jul 3, 2019
@ghost ghost deleted the rybrande/Templating/P7ReactReduxAuth branch July 3, 2019 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@javiercn javiercn javiercn approved these changes

@pranavkm pranavkm Awaiting requested review from pranavkm

+1 more reviewer

@mkArtakMSFT mkArtakMSFT mkArtakMSFT approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Milestone

3.0.0-preview7

Development

Successfully merging this pull request may close these issues.

5 participants

@ryanbrandenburg @javiercn @mkArtakMSFT @aspnet-hello