Develop #1
Develop #1
Conversation
(Chore) Further code refactoring
(Chore) Further code refactoring
There was a problem hiding this comment.
Hi Daniel nice work !! I have put some comment which seem to repeat themselves in most of the modules please pass through the other modules and try to rectify the similar logics that I have commented on
NB. have not finished going through all the modules especially the test. Will do so when I get Time cheers
| time_now = datetime.now() | ||
| ret_value = request.json | ||
|
|
||
| bucklist_name = '' if not ret_value else ret_value.get('name', '') |
There was a problem hiding this comment.
avoid repetition
if not ret_value should have been validated at point of assignment ret_value = request.json
i think bucketlist_name should only recieve the value
bucketlist_name=ret_value.get('name', '')
| bucketlist_names = [bucket.name for bucket in all_current_bucketlist] | ||
|
|
||
| # check if the name was passed or exists | ||
| if bucklist_name not in bucketlist_names: |
There was a problem hiding this comment.
- I don't think this logic is necessary the unique attribute in of Column in SQLALchemy can handle the e.g
Column('col1', Integer, unique=True),
once applied you can handle the Integrity error that will be thrown
- You may also try to query for the
bucklist_nameand check if the query returns an object
| date_modified=time_now, | ||
| created_by=g.user_id) | ||
| b_list.save() | ||
| new_items = BucketList.query.filter_by(date_created=time_now).first() |
There was a problem hiding this comment.
I want to obtain all the details of the newly created bucketlist. Am querying the database so to obtain new items id which is generated by the database.
| time_now = datetime.now() | ||
| return_bucketlists = {} | ||
|
|
||
| limit = '20' if not request.args else request.args.get('limit', '20') |
There was a problem hiding this comment.
The request object will always have an args attribute we can reduce the code to request.args.get('limit', '20')
There was a problem hiding this comment.
Am using request.args.get('limit', '20') because i believe there is a possibility that request.args may not be null but the key limit may be missing at a such an instance. So when such an occurence happens, i provide default value '20' and avoid KeyError exception being raised.
|
|
||
| all_bucks = BucketList.query.filter_by( | ||
| created_by=g.user_id).limit(limit).all() | ||
| q = '' if not request.args else request.args.get('q', '') |
There was a problem hiding this comment.
The request object will always have an args attribute we can reduce the code
| return return_response( | ||
| dict(Error=('Get Failed: No Bucketlists Found.')), 500) | ||
|
|
||
| for bucket_l in all_bucks: |
There was a problem hiding this comment.
Hi have a look at this link you might need to serialize your data from within the models.
| Error='Update Failed: Bucketlist Id %s ' | ||
| 'was not found' % id)), 400) | ||
|
|
||
| name = request.json.get('name', '') |
There was a problem hiding this comment.
use None instead of empty string ""
name = request.json.get('name', None)
There was a problem hiding this comment.
Okay that will also work, i will do that.
| return return_response(new_query.get(), 201) | ||
|
|
||
| if request.method == 'DELETE': | ||
| if not query: |
| query_items = Item.query.all() | ||
| item_names = [item.name for item in query_items] | ||
|
|
||
| # Helps not to save a name already saved or not found |
There was a problem hiding this comment.
use Unique constraint attribute on this field.
There was a problem hiding this comment.
The reason why a unique constraint may not be that relevant is that a user logged in the system cannot be able to view other people's bucketlist and other information. The Secret behind this that a single user cannot have several items with the same name but multiple users can have each a single entry of the item with the same name.
| if query_new: | ||
| query_new = query_new.get() | ||
| else: | ||
| query_new = dict(Message="Resource already exists") |
|
Changes Unknown when pulling a3a41cf on develop into * on master*. |
|
Changes Unknown when pulling 00a1a9d on develop into * on master*. |
|
Pull request Review Notes
|
|
|
Changes Unknown when pulling 3bc81fb on develop into * on master*. |
|
Changes Unknown when pulling 1145d3a on develop into * on master*. |
(Chore) adding more comments (BugFix) fix a bug with the items
|
Changes Unknown when pulling 140fff1 on develop into * on master*. |
No description provided.