Skip to content

Dotnetassign#28

Open
Deepakyadav19 wants to merge 13 commits into
cybergroupdevs:masterfrom
Deepakyadav19:dotnetassign
Open

Dotnetassign#28
Deepakyadav19 wants to merge 13 commits into
cybergroupdevs:masterfrom
Deepakyadav19:dotnetassign

Conversation

@Deepakyadav19

Copy link
Copy Markdown
Contributor

Pull request for webApi Project.

namespace WebApplication2.controllers
{
[Route("api/[controller]")]
public class ValuesController : Controller

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please remove unused code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -0,0 +1,9 @@
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please add bin folder to .gitignore

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ohkk.

}
// DELETE api/<controller>/5
[HttpDelete]
public ActionResult pdeleteemployee([FromBody]Custom_Models.tcompanycustom data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as the other controller. Don't need to pass data parameter only Id would suffice.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please Resolve the PR Comments

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

only using id for deletion.


// GET: api/<controller>
[HttpGet]
public ActionResult getStudent([FromBody]Custom_Models.tcompanycustom data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove unused parameters.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please Resolve PR Comments

@@ -0,0 +1,73 @@
#pragma checksum "C:\Users\deepak.yadav1\source\repos\WebApplication2\WebApplication2\Pages\About.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "ee9e3a2b396caec39857098be8c15f8288603f03"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add obj folder in .gitignore file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks for reviewing aastha.


var claims = new[] {
new Claim(JwtRegisteredClaimNames.Sub, _configuration["Jwt:Issuer"]),
new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is the use of GUID in JWT please explain.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in this project it is not required because we are running the project on same device..it is useful for generating a new unique string for every device...

}
// DELETE api/<controller>/5
[HttpDelete]
public ActionResult pdeleteemployee([FromBody]Custom_Models.tcompanycustom data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please Resolve the PR Comments


// GET: api/<controller>
[HttpGet]
public ActionResult getStudent([FromBody]Custom_Models.tcompanycustom data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please Resolve PR Comments

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.

3 participants