Conversation
Co-authored-by: A Anunaya <76819712+Anunaya07@users.noreply.github.com> Co-authored-by: Aksh Bansal <63552235+Aksh-Bansal-dev@users.noreply.github.com>
Priyansh61
suggested changes
Apr 12, 2024
Comment on lines
170
to
174
| if not check.exists(): | ||
| messages.error(request,"Request not added, no such project investigator exists 2") | ||
| messages.error(request,"Request not added, no such project investigator exists ") | ||
| return render(request,"rs/projects.html") | ||
|
|
||
|
|
There was a problem hiding this comment.
This is being repeated twice kindly take a look....
Contributor
There was a problem hiding this comment.
I think one is checking for pid and another is checking copid
Comment on lines
193
to
199
| for i in obj: | ||
| if i.project_name==projectname: | ||
| messages.error(request,"Request not added, project name already exists") | ||
| return render(request,"rs/projects.html") | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Use better variable naming here for better understanding of this part
Comment on lines
137
to
147
| def add_projects(request): | ||
|
|
||
| # designation = getDesignation(request.user.username) | ||
| # print("designation is " + designation) | ||
| # if designation != 'rspc_admin': | ||
| # messages.error(request, 'Only RSPC Admin can add projects') | ||
| # return redirect("/research_procedures") | ||
|
|
||
| if request.method== "POST": | ||
| obj= request.POST | ||
| projectname= obj.get('project_name') |
There was a problem hiding this comment.
Please break this function into modular small function since it has a very important functionality and is quite big
theoden42
reviewed
Apr 12, 2024
theoden42
left a comment
theoden42
left a comment
There was a problem hiding this comment.
Please provide clarification on the view_inbox part, other than that LGTM
| # data = view_inbox(request.user.username,user_designation, "research_procedures") | ||
| user_obj = get_user_by_username(request.user.username) | ||
|
|
||
| data= Tracking.objects.filter(receiver_id=user_obj, receive_design=user_designation, file_id__src_module="research_procedures") |
There was a problem hiding this comment.
why use this instead of view_inbox which directly provides a list of file ids?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue that this pull request solves
responsiveness added and error handling done
Closes: # (issue number)
Proposed changes
1.error handling done
2. user can only do access pages after login
Brief description of what is fixed or changed
Types of changes
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that applyScreenshots
https://drive.google.com/drive/folders/1G07daQnqNSvfMfnX0TgyTTS6RUTECYmO
Please attach the screenshots of the changes made in case of change in user interface
Other information
Any other information that is important to this pull request
@varun7singh
@ChaudharyRaman
@prajjwalkapoor