camusicjunkie / powerlfm Goto Github PK

When a track is currently playing NowPlaying is set to $true. When Get-LFMUserRecentTrack is called with a limit of 1, two tracks are displayed.

Change structure of code below and add another check when limit equals 1.

if ($PSBoundParameters.ContainsKey('EndDate') -and $track.'@attr'.NowPlaying -eq 'true') {
    $trackInfo = $trackInfo[1]
}

Correct spelling for ValueFromReminingArguments in all interface tests. Should be ValueFromRemainingArguments

Create a simple form using winforms instead of Internet Explorer when requesting a new token from the Last.fm API

https://www.last.fm/api/scrobbling#now-playing-requests

I don't see the lib folder, let alone the DLL in the download or in the repo. Am I missing something? Running Win10, PS v5.1.

Thanks

All functions that have a verb that could change system state like Add, New, or Set should have the supportsShouldProcess functionality.

Chang the call to the signing functions to use splatting throughout.

Integrating Microsoft.Powershell.SecretManagement will greatly simplify how credentials are stored and retrieved for the module.

This affects the following functions:

Get-LFMUserTopAlbum
Get-LFMUserTopArtist
Get-LFMUserTopTrack

Move from the registry to using the Windows Password Vault to store the API token, session key, and shared secret. This is mostly to appease PSScriptAnalyzer. Using ConvertTo-SecureString $TextToEncrypt -AsPlainText -Force isn't best practice.

These tests shouldn't have to account for whether the Last.fm API is actually working. The integration tests will probably get removed as the API should already be tested on their end.

When using the start and end date parameters the dates are only displayed a week at a time. The weeks start and end at the same time for every week which will always be before whatever song is now playing.

Some functions have incorrect types. For example some parameters are set up as so...
[string] Id or [string] Limit. They should be [guid] Id or [int] Limit respectively.

This will break the interface tests for all functions where the type is changed. All those tests will have to be updated.

Add Pester tests for each public function.

Change It block format to check for a matching string instead of checking for true.

Current:

It 'Should contain an output type of PowerLFM.Album.Info' {
    $command.OutputType.Name -contains 'PowerLFM.Album.Info' | Should -BeTrue
}

Proposed:

It 'Should contain an output type of PowerLFM.Album.Info' {
    $command.OutputType.Name | Should -Be 'PowerLFM.Album.Info'
}

Change format of it block when testing for parameter set name. Use assertion contain instead of betrue.

Current:

It 'Should have a parameter set of album' {
    $command.ParameterSets.Name -contains 'album' | Should -BeTrue
}

Proposed:

It 'Should have a parameter set of album' {
    $command.ParameterSets.Name | Should -Contain 'album'
}

Add functionality for the rest of the track methods.

Thanks for your module, I was going to try something similar to show/notify me of any new albums put out by my favourite artists. (I will export a list of artists with powershell eg. get-childitem -path Z:\MUSIC)

It would be cool to have something like $artist.latestalbum which could include year, so it could be filtered.

Thoughts? Cheers

If there is currently no connection to Last.fm when this function is run it should cache all tracks played. It should cache them in a persistent way that will survive restarts. Maybe a simple text file in the user's AppData folder.

When this function is run, and the connection to Last.fm is reestablished, it should check for this file and scrobble all tracks it contains.

If the username parameter is not used this function should use the authenticated user as a parameter to the method call.

Add external help file for each public function.

Remove mandatory attribute from parameter block for UserName parameter. Add key for the session key to the apiParams hashtable. See below for example:

begin {
    $apiParams = @{
        'method' = 'user.GetInfo'
        'api_key' = $LFMConfig.APIKey
        'sk' = $LFMConfig.SessionKey
        'format' = 'json'
    }
}

There are some instances where the error returned in $_.ErrorDetails.Message is not json. This causes ConvertFrom-Json to error out and not actually return the correct error.

Currently the SecretManagement module is a prerelease so it can't be set as a required module in PowerLFM and be downloaded automatically from the PowershellGallery when PowerLFM is installed. Because of this it will be packaged with PowerLFM to not require it to be installed separately by the user.

Raise the allowed value of the Limit parameter to 200. In the official API documentation, this is the maximum value https://www.last.fm/api/show/user.getRecentTracks

I moved these files out under the main repo folder just to help with testing locally knowing that's not how it should really be done. Moving them back in anticipation of creating a build process to compile everything to what will eventually be published to the PSGallery.

These properties were removed from the user.getFriend api. This information can also be found in Get-LFMUserInfo and Get-LFMUserRecentTrack.

Example from Get-LFMAlbumInfo. Move most of this code to a new function. Pass in $PSBoundParameters to new function.

begin {
    $apiParams = @{
        'method' = 'album.getInfo'
        'api_key' = $LFMConfig.APIKey
        'format' = 'json'
    }

    switch ($PSBoundParameters.Keys) {
        'UserName' {$apiParams.Add('username', $UserName)}
        'AutoCorrect' {$apiParams.Add('autocorrect', 1)}
    }
}
process {
    switch ($PSCmdlet.ParameterSetName) {
        'album' {$apiParams.Add('artist', $Artist);
                    $apiParams.Add('album', $Album)}
        'id'    {$apiParams.Add('mbid', $Id)}
    }

    #Building string to append to base url
    $keyValues = $apiParams.GetEnumerator() | ForEach-Object {
        "$($_.Name)=$($_.Value)"
    }
    $string = $keyValues -join '&'

    $apiUrl = "$baseUrl/?$string"
}

FindAllByUserName("$UserName") in Add-LFMVaultPass will always throw an exception when no results are found for the user, such as the first time the module is run.
Microsoft doc: https://docs.microsoft.com/en-us/uwp/api/windows.security.credentials.passwordvault.findallbyusername

Try RetrieveAll method with Where-Object instead:
if (($vault.RetrieveAll() | Where-Object {$_.UserName -eq $UserName}).Count -ne 0)

This is mostly to consolidate all interface tests from each function to a single set of interface tests. It uses a json file to reference what each function should look like.

This affects: Get-LFMUserWeeklyAlbumChart, Get-LFMUserWeeklyArtistChart, Get-LFMUserWeeklyTrackChart, and Get-LFMUserWeeklyChartList

Changed From to StartDate.
Changed To to EndDate.

New-MarkdownHelp has been run against every function. Now each function needs to populated with its specific details.

All functions that utilize the Id parameter should be set to mandatory within its parameter set. This makes all functions consistent. Also making small tweaks and adding the int type to the output on some functions.

The startdate and enddate parameters are evaluated in the begin block. They should be in the process block so this function can properly accept pipeline input.

The successful authenticated method calls weren't returning json when json was selected as the format. They seemed to only return xml. Changing the functions with these authenticated calls to use Invoke-WebRequest and the default format of xml.

The $hash variable declared in the end block needs to be updated for the older functions. Need to remove the extra processing. Invoke-WebRequest will be replaced by Invoke-RestMethod.