Maximezmt/music Itunes Api #45

MaximeZmt · 2022-03-08T23:08:28Z

Music API

This is related to issue #45

This Pull Requests contains:

ch.sdp.vibester.api: ItunesMusicApi: an api with a function querySong(...) that will return a song object
ch.sdp.vibester.model: Song: a class that represent a song containing differents attributes
ch.sdp.vibester: MusicTemporary: An activity that is used to test the music API

In addition the code is tested with a coverage of: [100]%

Notes for reviewers

The minapk has increased of one (to 24) to use the completableFuture API
The Readme has been updated with new Kamila username

jiabaow

Excellent work for both features and tests!
I left a few suggestions for clarity but it's already in pretty good shape

jiabaow · 2022-03-11T05:52:16Z

app/src/androidTest/java/ch/sdp/vibester/MusicTemporaryTest.kt

+            val intent =
+                Intent(ApplicationProvider.getApplicationContext(), MusicTemporary::class.java)
+            val scn: ActivityScenario<GreetingActivity> = ActivityScenario.launch(intent)
+            Espresso.onView(ViewMatchers.withId(R.id.musicName))


Suggested change

Espresso.onView(ViewMatchers.withId(R.id.musicName))

onView(withId(R.id.musicName))

This should work too (depends on your import) and it will be more consistent with the rest

Done in my new commit :)

jiabaow · 2022-03-11T05:54:12Z

app/src/androidTest/java/ch/sdp/vibester/MusicTemporaryTest.kt

+            val scn: ActivityScenario<GreetingActivity> = ActivityScenario.launch(intent)
+            Espresso.onView(ViewMatchers.withId(R.id.musicName))
+                .perform(ViewActions.typeText(inputName), ViewActions.closeSoftKeyboard())
+            Espresso.onView(ViewMatchers.withId(R.id.validate)).perform(ViewActions.click())


Suggested change

Espresso.onView(ViewMatchers.withId(R.id.validate)).perform(ViewActions.click())

onView(withId(R.id.validate)).perform(ViewActions.click())

jiabaow · 2022-03-11T05:54:26Z

app/src/androidTest/java/ch/sdp/vibester/MusicTemporaryTest.kt

+            Espresso.onView(ViewMatchers.withId(R.id.musicName))
+                .perform(ViewActions.typeText(inputName), ViewActions.closeSoftKeyboard())
+            Espresso.onView(ViewMatchers.withId(R.id.validate)).perform(ViewActions.click())
+            Espresso.onView(ViewMatchers.withId(R.id.textViewPlaying))


Suggested change

Espresso.onView(ViewMatchers.withId(R.id.textViewPlaying))

onView(withId(R.id.textViewPlaying))

jiabaow · 2022-03-11T05:58:00Z

app/src/main/java/ch/sdp/vibester/api/ItunesMusicApi.kt

+import android.os.Build
+import android.util.Log
+import ch.sdp.vibester.model.Song
+//import com.android.volley.Request


Suggested change

//import com.android.volley.Request

Done in the new commit

jiabaow · 2022-03-11T05:58:08Z

app/src/main/java/ch/sdp/vibester/api/ItunesMusicApi.kt

+import android.util.Log
+import ch.sdp.vibester.model.Song
+//import com.android.volley.Request
+//import com.android.volley.toolbox.StringRequest


Suggested change

//import com.android.volley.toolbox.StringRequest

jiabaow · 2022-03-11T05:58:16Z

app/src/main/java/ch/sdp/vibester/api/ItunesMusicApi.kt

+import ch.sdp.vibester.model.Song
+//import com.android.volley.Request
+//import com.android.volley.toolbox.StringRequest
+//import com.android.volley.toolbox.Volley


Suggested change

//import com.android.volley.toolbox.Volley

jiabaow · 2022-03-11T06:00:17Z

app/src/main/res/layout/activity_music_temporary.xml

+        android:layout_marginTop="208dp"
+        android:layout_marginEnd="111dp"
+        android:layout_marginBottom="23dp"
+        android:text="Enter a Music Name:"


consider using @string instead of hardcoded string

Yes, but as it is a temporary activity, I didn't put it in the String.xml otherwise It will flood it with not important value :)

jiabaow · 2022-03-11T06:00:31Z

app/src/main/res/layout/activity_music_temporary.xml

+        android:layout_marginEnd="89dp"
+        android:layout_marginBottom="28dp"
+        android:ems="10"
+        android:hint="Music Name"


jiabaow · 2022-03-11T06:00:39Z

app/src/main/res/layout/activity_music_temporary.xml

+        android:layout_marginTop="28dp"
+        android:layout_marginEnd="159dp"
+        android:layout_marginBottom="29dp"
+        android:text="Validate"


jiabaow · 2022-03-11T06:00:49Z

app/src/main/res/layout/activity_music_temporary.xml

+        android:layout_marginTop="15dp"
+        android:layout_marginEnd="179dp"
+        android:layout_marginBottom="267dp"
+        android:text="Playing: None"


MaximeZmt · 2022-03-11T06:20:21Z

Excellent work for both features and tests! I left a few suggestions for clarity but it's already in pretty good shape

I've also added javadoc that was not done before 😅 Now It's all good 👍

codeclimate · 2022-03-11T06:33:09Z

Code Climate has analyzed commit 51d5918 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 99.4% (0.2% change).

View more on Code Climate.

zwierski

Looks good to me, the code is very clear.

MaximeZmt added 3 commits March 6, 2022 15:59

temporary version of api music

2c7591a

refactor music api

bcb63df

refactor music api#2 small correction

7a6d6a5

MaximeZmt self-assigned this Mar 8, 2022

MaximeZmt and others added 16 commits March 9, 2022 00:14

Merge branch 'main' into maximezmt/musicApi

630316c

update test files

b8f13db

update directory architecture

5077a04

fix broken test

0164c49

update song test error

04e4b29

adding temporary music activity

971c982

fix uselessTest

d5613df

attempt make it pass CI

6b2db5d

New test for ItunesMusicApi

3f527c1

test fixes

c5377cb

test update #1

4d8d89f

test update #2

97ded4c

test update #3

e19265f

remove mockito attempt

b57eb4b

correction of test

3a8e896

tempo upgrade from volley to okhttp

094af84

MaximeZmt marked this pull request as ready for review March 10, 2022 21:23

MaximeZmt and others added 3 commits March 10, 2022 23:11

release of api

0045705

removed change of misc.xml

ec34088

Merge branch 'main' into maximezmt/musicApi

07f516c

MaximeZmt requested review from jiabaow, kamilababayeva, laurislopata, Tsathogguaa and zwierski March 10, 2022 22:19

MaximeZmt added the actual: 7h label Mar 10, 2022

jiabaow approved these changes Mar 11, 2022

View reviewed changes

adding javadoc

03350f7

Update MusicTemporaryTest format

51d5918

MaximeZmt added actual: 8h and removed actual: 7h labels Mar 11, 2022

zwierski approved these changes Mar 11, 2022

View reviewed changes

MaximeZmt merged commit 7737c55 into main Mar 11, 2022

MaximeZmt deleted the maximezmt/musicApi branch March 11, 2022 07:39

This was linked to issues Mar 11, 2022

API: find a music api that work well (#20) #33

Closed

API: Create a basic class for a music server (#20) #32

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maximezmt/music Itunes Api #45

Maximezmt/music Itunes Api #45

MaximeZmt commented Mar 8, 2022 •

edited

Loading

jiabaow left a comment

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

MaximeZmt Mar 11, 2022

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

MaximeZmt Mar 11, 2022

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

MaximeZmt Mar 11, 2022

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

jiabaow Mar 11, 2022

MaximeZmt commented Mar 11, 2022

codeclimate bot commented Mar 11, 2022

zwierski left a comment

	Espresso.onView(ViewMatchers.withId(R.id.musicName))
	onView(withId(R.id.musicName))

	Espresso.onView(ViewMatchers.withId(R.id.validate)).perform(ViewActions.click())
	onView(withId(R.id.validate)).perform(ViewActions.click())

	Espresso.onView(ViewMatchers.withId(R.id.textViewPlaying))
	onView(withId(R.id.textViewPlaying))

Maximezmt/music Itunes Api #45

Maximezmt/music Itunes Api #45

Conversation

MaximeZmt commented Mar 8, 2022 • edited Loading

Music API

This is related to issue #45

Notes for reviewers

jiabaow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaximeZmt commented Mar 11, 2022

codeclimate bot commented Mar 11, 2022

zwierski left a comment

Choose a reason for hiding this comment

MaximeZmt commented Mar 8, 2022 •

edited

Loading