Skip to content

Add the option to raise issue when license submission failed #320

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

Open
Ugtan wants to merge 2 commits into
base: python3
Choose a base branch
from
Open

Add the option to raise issue when license submission failed #320

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0441b3e
Add the option to raise issue when license submission failed
Jul 21, 2021
0fd254f
Make requested changes
Ugtan Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/app/templates/app/submit_new_license.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,11 +459,23 @@
}
}
else{
var warningMessage = "Please note that there was a problem opening the issue for the SPDX legal team. Please email spdx-legal@lists.spdx.org with SPDX ID for the license you are submitting";
$("#messages").html('<div class="alert alert-warning alert-dismissable fade in"><a href="#" class="close" data-dismiss="alert" aria-label="close">&times;</a><strong>Warning! </strong>'+ warningMessage +'</div>');
setTimeout(function() {
$("#messages").html("");
}, 7000);
var errorMessage = 'Error occured while submitting the license. Please contact to spdx-legal@lists.spdx.org with SPDX ID for submitting the license. Would you like to submit this problem to github so that the developers can look at it?';
Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 7, 2021

Choose a reason for hiding this comment

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

Please contact spdx-legal@lists.spdx.org

Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 7, 2021

Choose a reason for hiding this comment

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

Can we have constant variable with these message which can be referenced here?

Copy link
Copy Markdown
Collaborator Author

@Ugtan Ugtan Aug 8, 2021

Choose a reason for hiding this comment

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

Could you elaborate a bit please?

Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 8, 2021

Choose a reason for hiding this comment

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

Having all these string message in one file and then referencing here in javascript.

$("#modal-header").removeClass("yellow-modal green-modal");
$("#modal-header").addClass("red-modal");
$(".modal-footer").html(`<button class="btn btn-primary btn-space" id="submiterrorissue"><span class="glyphicon glyphicon-ok"></span> Yes</button><button class="btn btn-primary btn-space" id="no"><span class="glyphicon glyphicon-remove"></span> No</button>`);
$("#modal-body").html(errorMessage);
$("#myModal").modal({
backdrop: 'static',
keyboard: true,
show: true
});
$(document).on('click','button#submiterrorissue', function(event){
$("#myModal").modal("hide");
makeIssue(data);
});
$(document).on('click','button#no', function(event){
$("#myModal").modal("hide");
});
}
$("#fullname").val("");
$("#shortIdentifier").val("");
Expand Down Expand Up @@ -549,7 +561,7 @@
success: function (data) {
var githubCode = data.statusCode;
if(githubCode == '201'){
var successMessage = 'The issue "' + msg + '" has been successfully submitted.';
var successMessage = 'The issue has been successfully submitted.';
$("#messages").html('<div class="alert alert-success alert-dismissable fade in"><a href="#" class="close" data-dismiss="alert" aria-label="close">&times;</a><strong>Success! </strong>'+ successMessage +'</div>');
setTimeout(function() {
$("#messages").html("");
Expand Down
15 changes: 15 additions & 0 deletions src/app/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,21 @@ def createIssue(licenseAuthorName, licenseName, licenseIdentifier, licenseCommen
return r.status_code


def createErrorIssue(occured_at, made_by, error_message, license_text, token):
""" View for creating an GitHub issue
with the complete Error Report to the
SPDX Online tools repository.
"""

body = "**1.** Occured At: {0}\n**2.** Made By: {1}\n**3.** Error Message: {2}\n**4.** License Text: {3}\n".format(occured_at, made_by, error_message, license_text)
Copy link
Copy Markdown

@SantiagoTorres SantiagoTorres Jul 22, 2021

Choose a reason for hiding this comment

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

I wonder if it's easier to do this with a multiline string

Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 7, 2021

Choose a reason for hiding this comment

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

Yes I would recommend to use multiline string

title = "Error Report"
Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 7, 2021

Choose a reason for hiding this comment

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

Lets not directly assign a title. Keep a string constant. Similary for url - construct it from other url or keep it as setting in settings.py

Copy link
Copy Markdown
Collaborator Author

@Ugtan Ugtan Aug 8, 2021

Choose a reason for hiding this comment

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

I kept the url as setting in the settings.py. What about the title of the issue? Should we just kept it blank?

Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 8, 2021

Choose a reason for hiding this comment

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

Title should included license name/id

payload = {'title' : title, 'body': body}
headers = {'Authorization': 'token ' + token}
url = "https://api.github.com/repos/spdx/spdx-online-tools/issues"
Copy link
Copy Markdown

@SantiagoTorres SantiagoTorres Jul 22, 2021

Choose a reason for hiding this comment

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

Looking around it seems that this URL is constructed out of the settings on top, we perhaps should follow the pattern they use.

Not fully relevant to this PR, but I would assume you can combine a bunch of things into a single "submit-to-api" function...

r = requests.post(url, data=json.dumps(payload), headers=headers)
return r.status_code


def postToGithub(message, encodedContent, filename):
""" Function to create a new file on with encodedContent
"""
Expand Down
77 changes: 44 additions & 33 deletions src/app/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,9 @@ def submitNewLicense(request):
""" Other errors raised """
logger.error(str(format_exc()))
if (request.is_ajax()):
ajaxdict["type"] = "error"
ajaxdict["data"] = "Unexpected error, please email the SPDX technical workgroup that the following error has occurred: " + format_exc()
response = dumps(ajaxdict)
return HttpResponse(response,status=500)
ajaxdict['error'] = str(format_exc())
ajaxdict['inputlicensetext'] = request.POST['text']
return JsonResponse(ajaxdict)
Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 7, 2021

Choose a reason for hiding this comment

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

Does the caller expects "type" and "data" field now?

Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 15, 2021

Choose a reason for hiding this comment

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

@Ugtan What about this?

Copy link
Copy Markdown
Collaborator Author

@Ugtan Ugtan Aug 15, 2021

Choose a reason for hiding this comment

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

@rtgdk No. I had to use Json response to get the data after making a ajax request in order to give them the ability to choose if they want to submit it or not.

return HttpResponse("Unexpected error, please email the SPDX technical workgroup that the following error has occurred: " + format_exc(), status=500)
else:
email=""
Expand Down Expand Up @@ -1101,35 +1100,47 @@ def issue(request):
try:
github_login = user.social_auth.get(provider='github')
token = github_login.extra_data["access_token"]
licenseAuthorName = request.POST['licenseAuthorName']
licenseName = request.POST['licenseName']
licenseIdentifier = request.POST['licenseIdentifier']
licenseOsi = request.POST['licenseOsi']
licenseSourceUrls = request.POST.getlist('licenseSourceUrls')
licenseExamples = request.POST.getlist('exampleUrl')
licenseHeader = request.POST['licenseHeader']
licenseComments = request.POST['comments']
licenseText = request.POST['inputLicenseText']
userEmail = request.POST['userEmail']
licenseNotes = request.POST['licenseNotes']
listVersionAdded = request.POST['listVersionAdded']
matchId = request.POST['matchIds']
diffUrl = request.POST['diffUrl']
msg = request.POST.get('msg', None)
urlType = utils.NORMAL
data = {}
xml = generateLicenseXml(licenseOsi, licenseIdentifier, licenseName,
listVersionAdded, licenseSourceUrls, licenseHeader, licenseNotes, licenseText)
now = datetime.datetime.now()
licenseRequest = LicenseRequest(licenseAuthorName=licenseAuthorName, fullname=licenseName, shortIdentifier=licenseIdentifier,
submissionDatetime=now, userEmail=userEmail, notes=licenseNotes, xml=xml)
licenseRequest.save()
licenseRequestId = licenseRequest.id
serverUrl = request.build_absolute_uri('/')
licenseRequestUrl = os.path.join(serverUrl, reverse('license-requests')[1:], str(licenseRequestId))
statusCode = utils.createIssue(licenseAuthorName, licenseName, licenseIdentifier, licenseComments, licenseSourceUrls, licenseHeader, licenseOsi, licenseExamples, licenseRequestUrl, token, urlType, matchId, diffUrl, msg)
data['statusCode'] = str(statusCode)
return JsonResponse(data)

if request.POST.get('error',None):
Copy link
Copy Markdown
Collaborator

@rtgdk rtgdk Aug 7, 2021

Choose a reason for hiding this comment

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

Move the main part to utils.py

occured_at = datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d")
made_by = github_login.extra_data['login']
error_message = request.POST['error']
license_text = request.POST['inputlicensetext']
data = {}
statusCode = utils.createErrorIssue(occured_at, made_by, error_message, license_text, token)
data['statusCode'] = str(statusCode)
return JsonResponse(data)
else:
licenseAuthorName = request.POST['licenseAuthorName']
licenseName = request.POST['licenseName']
licenseIdentifier = request.POST['licenseIdentifier']
licenseOsi = request.POST['licenseOsi']
licenseSourceUrls = request.POST.getlist('licenseSourceUrls')
licenseExamples = request.POST.getlist('exampleUrl')
licenseHeader = request.POST['licenseHeader']
licenseComments = request.POST['comments']
licenseText = request.POST['inputLicenseText']
userEmail = request.POST['userEmail']
licenseNotes = request.POST['licenseNotes']
listVersionAdded = request.POST['listVersionAdded']
matchId = request.POST['matchIds']
diffUrl = request.POST['diffUrl']
msg = request.POST.get('msg', None)
urlType = utils.NORMAL
data = {}
xml = generateLicenseXml(licenseOsi, licenseIdentifier, licenseName,
listVersionAdded, licenseSourceUrls, licenseHeader, licenseNotes, licenseText)
now = datetime.datetime.now()
licenseRequest = LicenseRequest(licenseAuthorName=licenseAuthorName, fullname=licenseName, shortIdentifier=licenseIdentifier,
submissionDatetime=now, userEmail=userEmail, notes=licenseNotes, xml=xml)
licenseRequest.save()
licenseRequestId = licenseRequest.id
serverUrl = request.build_absolute_uri('/')
licenseRequestUrl = os.path.join(serverUrl, reverse('license-requests')[1:], str(licenseRequestId))
statusCode = utils.createIssue(licenseAuthorName, licenseName, licenseIdentifier, licenseComments, licenseSourceUrls, licenseHeader, licenseOsi, licenseExamples, licenseRequestUrl, token, urlType, matchId, diffUrl, msg)
data['statusCode'] = str(statusCode)
return JsonResponse(data)
Copy link
Copy Markdown

@SantiagoTorres SantiagoTorres Jul 22, 2021

Choose a reason for hiding this comment

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

I wonder if you can simplify the code by moving the return outside of the if.

Copy link
Copy Markdown

@SantiagoTorres SantiagoTorres Jul 22, 2021

Choose a reason for hiding this comment

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

I think you can actually avoid a bunch of repetition by moving other things out of this if.

Copy link
Copy Markdown
Collaborator Author

@Ugtan Ugtan Jul 23, 2021

Choose a reason for hiding this comment

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

You're right. I will do it.


except UserSocialAuth.DoesNotExist:
""" User not authenticated with GitHub """
if (request.is_ajax()):
Expand Down