Skip to content
Snippets Groups Projects

Implementation of Project Week 3

Merged Implementation of Project Week 3
week3 into master
5 unresolved threads
Merged yuhou2 requested to merge week3 into master
5 unresolved threads

Create the login and sign-in page. Connect the front end to the database. Create the user profile and library page. Connect the library page to the albums from the dashboard.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
music-search-library-app/src/components/AlbumsList.js
44 href={album.external_urls.spotify}
45 rel="noopener noreferrer"
46 className="card-image-link"
47 >
48 {!_.isEmpty(album.images) ? (
49 <Card.Img
50 variant="top"
51 src={album.images[0].url}
52 alt="" />
53 ) : (
54 <img src={music} alt="" />
55 )}
56 </a>
57 <Card.Header>{album.name}</Card.Header>
58 <Card.Body>
59 <Card.Title>{album.artists.map((artist) => artist.name).join(', ')}</Card.Title>
  • yuefeng3
    yuefeng3 @yuefeng3 ·
    Developer

    I think it will be easier to read if you put <Card.Title> and </Card.Title> in different lines when the content is long, which will improve readability.

  • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/Login.js 0 → 100644
    13 const { email, password } = formData;
    14
    15 const onChange = e =>
    16 setFormData({ ...formData, [e.target.name]: e.target.value });
    17
    18 const onSubmit = e => {
    19 e.preventDefault();
    20 login(email, password);
    21 };
    22
    23 if (isAuthenticated) {
    24 alert("log in success")
    25 return <Redirect to="/dashboard" />;
    26 }
    27
    28 // const handleSumbit = (evt) => {
    • yuefeng3
      yuefeng3 @yuefeng3 ·
      Developer

      You might want to remove unused commented code in your submission to improve clarity, these code might create confusion for other people when trying to maintain or extend you code.

      Edited by yuefeng3
    • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/UserLibrary.js 0 → 100644
    24 boxShadow: '0px 5px 8px -3px rgba(0,0,0,0.14)',
    25 "&:focus": {
    26 borderRadius: 12,
    27 background: 'white',
    28 borderColor: deepPurple[500]
    29 },
    30 },
    31 paper: {
    32 padding: theme.spacing(2),
    33 textAlign: 'center',
    34 color: theme.palette.text.secondary,
    35 },
    36
    37 }));
    38
    39 function UserLibrary({ auth: { user }, loadUser }) {
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/UserProfile.js 0 → 100644
    65 alert("name change success")
    66 }
    67 if (values.password) {
    68 getData('/user/me/password', 'PUT', myHeaders, JSON.stringify({ 'password': values.password }))
    69 alert("password change success")
    70 }
    71
    72 }
    73
    74 useEffect(() => {
    75 getData('/user/me', 'GET', myHeaders, null)
    76 }, []);
    77
    78
    79 return (
    80
    • yuefeng3
      yuefeng3 @yuefeng3 ·
      Developer

      I noticed that there are many unnecessary space or line change through your code. You might want to install some plugins in your ide to automatically adjust tiny style issues for you.

    • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/utils/api.js 0 → 100644
    1 import axios from 'axios';
    2 import store from '../store/store';
    3 import { LOGOUT } from '../actions/types';
    4
    5 const api = axios.create({
    6 baseURL: '/',
    7 headers: {
    8 'Content-Type': 'application/json'
    9 }
    10 });
    11 /**
    12 intercept any error responses from the api
    • yuefeng3
      yuefeng3 @yuefeng3 ·
      Developer

      It is great to see you include detailed description for your APIs, which helps significantly not only for code review, but also for potential user to understand how to use your API.

    • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 ·
    Developer

    Peer Review Report

    1-This question references the Code Review Checklist available at course website. Select one item from Item 4 (Non Functional requirements) in the checklist, and answer the following questions:

    • Maintainability
    • Did well in some part of the code, and there are space for improvement.
    • Could include some comments in files like UserLibrary.js to improve readability. There are also many commented code which might create confusion for others.

    2-Please select one item from the Item 5 (OOAD Principle), and answer the following questions:

    • Single Responsibility Principle
    • Yes
    • Code was well separated into different files according to their responsibility. Good job!

    3-If applicable, does the author address all the comments you made last week?

    • Author did put more considerations into error status codes, but did not see any changes in unit tests.
  • yuhou2 approved this merge request

    approved this merge request

  • yuhou2 merged

    merged

  • yuhou2 mentioned in commit a4b0841b

    mentioned in commit a4b0841b

    • Please register or sign in to reply