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

Notify about the limit of 4096 named maps in Builder #16368

Merged

Conversation

moicalcob
Copy link
Collaborator

Resources

Context

  • There are several users that have reached their named map limit, and they can't publish new maps. This error wasn't being managed from cartodb, so we've added a check to delete old named maps if the user is reaching the limit, and Support will get notified about that.

Changes

  • Remove old named maps when the Maps API notify cartodb that the user is reaching the limit.
  • Notify support-internal that there is a user reaching the limit.

@moicalcob moicalcob requested a review from Shylpx October 29, 2021 15:47
@moicalcob moicalcob self-assigned this Oct 29, 2021
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #178449: Notify about the limit of 4096 named maps in Builder.

app/mailers/reporter_mailer.rb Outdated Show resolved Hide resolved
lib/carto/named_maps/api.rb Outdated Show resolved Hide resolved
lib/carto/named_maps/api.rb Outdated Show resolved Hide resolved
tables = Carto::UserTable.where(user_id: user.id, privacy: 0).limit(100).order('updated_at')
named_maps_ids = tables.map { |t| "tpl_#{t.visualization.id.tr('-', '_')}" }

named_maps_ids.each do |id|
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know if this method is executed synchronously in some point? I mean, if this is executed for example when a map is published, and the UI is waiting for a response, this will delay a lot the response.

If that is the case, my recommendations would be to create a new Resque worker class, and just enqueue it here. So it can be processed in the background.

Let me know if you have any question :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay! I'll try to do it, as it's executed synchronously 🤔 But I can't promise you that I'll be able to do it without any help 😅

lib/carto/named_maps/api.rb Outdated Show resolved Hide resolved
@Shylpx
Copy link
Collaborator

Shylpx commented Nov 4, 2021

Very nice job 🚀!! Just a few comments and doubts :)

Copy link
Collaborator

@Shylpx Shylpx left a comment

Choose a reason for hiding this comment

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

Very nice job! 🚀

app/mailers/reporter_mailer.rb Outdated Show resolved Hide resolved
@moicalcob moicalcob force-pushed the feature/sc-178449/notify-about-the-limit-of-4096-named-maps branch from 1cfdbf0 to f14947e Compare February 8, 2022 15:45
@moicalcob moicalcob merged commit 0456ec0 into master Feb 9, 2022
@moicalcob moicalcob deleted the feature/sc-178449/notify-about-the-limit-of-4096-named-maps branch February 9, 2022 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants