Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/del 2757 languages in sdk #256

Merged

Conversation

Sevitas
Copy link
Contributor

@Sevitas Sevitas commented Jan 13, 2021

Motivation

Introducing new listing endpoint for Languages in Delivery API and adding support for it in SDK

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

Check that method returns only active languages for project

@Sevitas Sevitas requested a review from Enngage as a code owner January 13, 2021 07:02
@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #256 (5d8df7d) into master (3f60637) will decrease coverage by 0.04%.
The diff coverage is 78.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #256      +/-   ##
==========================================
- Coverage   80.67%   80.63%   -0.05%     
==========================================
  Files         108      111       +3     
  Lines        1573     1611      +38     
  Branches      452      465      +13     
==========================================
+ Hits         1269     1299      +30     
  Misses         88       88              
- Partials      216      224       +8     
Impacted Files Coverage Δ
Kentico.Kontent.Delivery.Caching/CacheHelpers.cs 41.79% <20.00%> (-1.76%) ⬇️
...ico.Kontent.Delivery.Rx/DeliveryObservableProxy.cs 81.57% <50.00%> (-1.76%) ⬇️
Kentico.Kontent.Delivery/DeliveryClient.cs 66.88% <71.42%> (+0.21%) ⬆️
...co.Kontent.Delivery.Caching/DeliveryClientCache.cs 86.20% <85.71%> (-0.07%) ⬇️
....Delivery.Abstractions/DeliveryClientExtensions.cs 100.00% <100.00%> (ø)
...ivery/Languages/DeliveryLanguageListingResponse.cs 100.00% <100.00%> (ø)
Kentico.Kontent.Delivery/Languages/Language.cs 100.00% <100.00%> (ø)
...ent.Delivery/Languages/LanguageSystemAttributes.cs 100.00% <100.00%> (ø)
...ontent.Delivery/Urls/DeliveryEndpointUrlBuilder.cs 97.29% <100.00%> (+0.07%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f60637...5d8df7d. Read the comment docs.

@@ -3,28 +3,13 @@
namespace Kentico.Kontent.Delivery.Abstractions
{
/// <summary>
/// Represents system attributes of any object in Kentico Kontent.
/// Represents system attributes of ContentItem, ContentType and Taxonomy objects in Kentico Kontent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this abstraction should know nothing about the objects that implement it. therefore even this comment seems to be irrelevant.

@@ -65,5 +65,16 @@ public static Task<IDeliveryTaxonomyListingResponse> GetTaxonomiesAsync(this IDe
{
return client.GetTaxonomiesAsync(parameters);
}

/// <summary>
/// Returns languages.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we be a bit more specific? Something like "returns all active languages assigned to a given project and matching the optional filtering parameters."...

Kentico.Kontent.Delivery.Abstractions/IDeliveryClient.cs Outdated Show resolved Hide resolved
Kentico.Kontent.Delivery.Caching/CacheHelpers.cs Outdated Show resolved Hide resolved
@@ -235,6 +236,21 @@ public async Task<IDeliveryTaxonomyListingResponse> GetTaxonomiesAsync(IEnumerab
return new DeliveryTaxonomyListingResponse(response, taxonomies.ToList<ITaxonomyGroup>(), pagination);
}

/// <summary>
/// Returns languages.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, a better comment here would be appreciated.

@petrsvihlik
Copy link
Contributor

@petrsvihlik
Copy link
Contributor

tested with:

using Kentico.Kontent.Delivery.Abstractions;
using Kentico.Kontent.Delivery;
using System;
using Kentico.Kontent.Delivery.Builders.DeliveryClient;
using System.Threading.Tasks;
using Kentico.Kontent.Delivery.Urls.QueryParameters.Filters;
using System.Collections.Generic;

namespace ConsoleApp1
{
    class Program
    {
        static async Task Main(string[] args)
        {
            IDeliveryClient deliveryClient = DeliveryClientBuilder
                .WithOptions(o => o.WithProjectId("964d51ec-d07d-00cf-5aa5-7ee52ef970dc")
                .UseProductionApi()
                .WithCustomEndpoint("https://secret-endpoint.global.ssl.fastly.net/{0}")
                .Build()).Build();

            var lng = await deliveryClient.GetLanguagesAsync(new List<IQueryParameter>() { new EqualsFilter("system.name", "Czech")});

            foreach (var l in lng.Languages)
            {
                Console.WriteLine(l.System.Name);
            }
        }
    }
}

@petrsvihlik petrsvihlik merged commit 38de668 into kontent-ai:master Jan 18, 2021
@petrsvihlik petrsvihlik added this to the v15 milestone Jan 18, 2021
@petrsvihlik petrsvihlik added this to In progress in .NET Community Backlog via automation Jan 18, 2021
@petrsvihlik petrsvihlik moved this from In progress to Done in .NET Community Backlog Feb 8, 2021
@pokornyd pokornyd removed this from Done in .NET Community Backlog Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants